Antalya 26.8 - Forward port of #2290 - Plain merge tree partition export by arthurpassos · Pull Request #2548 · Altinity/ClickHouse

arthurpassos · 2026-10-12T13:57:23Z

Forward port of #2290

Changelog category (leave one):

New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Export partition on plain merge tree. Should close #1908

User facing & functional changes:

Changes how replicated partition task keys are encoded on zookeeper - this is not backwards compatible
Implements partition export for plain merge tree
Stop shipping empty parquet files on zero rows exports
New system.partition_exports table that covers both the previous replicated_partition_exports and the new one for plain merge tree

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

Regression jobs to run:

The combined export-partition port stopped before #2290, so plain `MergeTree` had no `EXPORT PARTITION` and only `system.replicated_partition_exports`. `getExternalMetadata` returns `std::shared_ptr` on this branch, so the shared Iceberg helper uses `dynamic_pointer_cast`.

chatgpt-codex-connector · 2026-10-12T13:57:37Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-12T14:17:59.769824Z	`aaf00db`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

github-actions · 2026-10-12T13:59:07Z

Workflow [PR], commit [5705da6]

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aaf00db011

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-12T14:17:55Z

    std::string getCompositeKey() const
    {
-        const auto qualified_table_name = QualifiedTableName {manifest.destination_database, manifest.destination_table};
-        return manifest.partition_id + "_" + qualified_table_name.getFullName();
+        return ExportPartitionUtils::compositeKey(
+            manifest.partition_id, manifest.destination_database, manifest.destination_table);


Preserve legacy Keeper keys during migration

On upgrade or in a mixed-version cluster, existing export children still use the legacy partition_id + "_" + qualified_table key, but this method now derives only the escaped dotted key. poll discovers the legacy child and inserts its manifest, while removeStaleEntries immediately compares this newly derived key against the actual legacy child name, cancels the local work, and erases the entry. The pending export is consequently stranded and a new export can be created under the new key for the same partition and destination; store the actual Keeper child key in the entry or migrate/recognize both encodings.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-12T14:17:55Z

+        if (const auto it = tasks.find(composite_key); it != tasks.end())
+        {
+            if (!force)
+                throw Exception(ErrorCodes::EXPORT_PARTITION_ALREADY_EXPORTED,
+                    "Export with key {} already exported or it is being exported. "
+                    "Set `export_merge_tree_partition_force_export` to overwrite it.",
+                    key_description);
+
+            previous_transaction_id = it->second.getDescriptor().transaction_id;
+        }


Reject force replacement while the old task is committing

If a forced export arrives after tryCommit has set committing but before its destination commit returns, this code replaces the registry entry anyway. killExportPart only cancels per-part workers, so the old Iceberg commit can still publish a snapshot under its old transaction ID; its result is then discarded because the registry contains the new ID, and the replacement task exports and commits the same files again under a different ID, duplicating rows. Handle committing like kill does—reject or defer replacement until the in-flight commit has finished.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-12T14:17:55Z

+    if (file_already_exists_policy != MergeTreePartExportFileAlreadyExistsPolicy::overwrite)
+    {
+        if (auto committed_paths = MultiFileStorageObjectStorageSink::tryReadCommittedPaths(
+                base_path, /* transaction_id= */ file_name, object_storage, local_context))
+        {
+            if (committed_paths->empty())
+                throw Exception(ErrorCodes::CORRUPTED_DATA, "Commit file for {} lists no data files", base_path);
+
+            return ImportResult::createAlreadyExported(std::move(*committed_paths));


Honor the error policy for committed parts

When a prior export's commit marker exists and export_merge_tree_part_file_already_exists_policy='error', this branch includes error among all non-overwrite policies and returns createAlreadyExported; ExportPartTask then reuses the paths and reports success. This directly contradicts the setting contract that error throws when a file already exists, preventing callers from detecting stale or conflicting output. Only skip should return createAlreadyExported, while error should raise FILE_ALREADY_EXISTS.

AGENTS.md reference: AGENTS.md:L157-L157

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-12T14:17:55Z

 void MultiFileStorageObjectStorageSink::commit()
 {
-    /// The constructor already ruled out a pre-existing commit file for every policy but
-    /// `overwrite`, so seeing one here means another exporter committed this part while we wrote.
-    if (file_already_exists_policy != FileAlreadyExistsPolicy::overwrite
-        && object_storage->exists(StoredObject(commit_file_path)))
-    {
-        throw Exception(ErrorCodes::FILE_ALREADY_EXISTS, "Commit file {} already exists, aborting {} export", commit_file_path, transaction_id);
-    }
-
    auto out = object_storage->writeObject(
        StoredObject(commit_file_path), 
        WriteMode::Rewrite, /* attributes= */


Recheck the commit marker before publishing it

For overlapping attempts of the same part—possible when a forced replacement cancels and removes an old worker while it is still writing—both attempts can observe no marker during import and begin writing. The removed commit-time existence check used to make the later finisher fail under error; now both unconditionally rewrite the marker and succeed, so the configured conflict policy is bypassed by a normal check-then-write race. Retain the policy in the sink and recheck the marker immediately before publishing it.

Useful? React with 👍 / 👎.

DimensionWieldr · 2026-10-14T23:58:56Z

@blau-ai

blau-ai · 2026-10-15T00:05:03Z

CI triage — @DimensionWieldr

Verdict: 7 red checks, 0 caused by this PR. Every failure is pre-existing on antalya-26.8, infra/baseline, or a job-level timeout. The checks that actually exercise this PR's feature are green — Regression release s3_export_part, s3_export_partition, iceberg_1/2, parquet* all pass, and the dedicated stateless/integration export tests pass (details below).

Evidence SHA: 5705da6 · run 38316133195

Not PR-related

Grype Scan (keeper: 4 high/crit, server-alpine: 1) — base-image/dependency CVE scan, independent of source changes. Fails identically on sibling PRs #2547 and #2558 (same 4/1 counts). → Pre-existing/infra. Address at branch level (base image bump), not in this PR.

Regression release swarms (1 module failed) — also fails on #2547 and #2558 (both 1 module (1 failed)). Known-broken Swarms suite on antalya-26.8. → Pre-existing. Safe to disregard for this PR; a re-run won't change it.

Regression release oauth (9/112 scenarios failed, 298/1814 retries failed) — OAuth is authentication-via-external-IdP; this PR touches only partition-export storage code (src/Storages/..., Iceberg writes, object-storage sinks) — zero overlap with auth/access control. The heavy retry-failure pattern is the signature of an external OAuth-provider dependency, not a product regression. → Not PR-caused (unrelated subsystem). Re-run; if it persists it's a branch/infra issue to track separately.

Integration tests (amd_asan_ubsan, db disk, old analyzer, 3/8) — 1/721
Failing test: test_s3_cluster/test.py::test_graceful_shutdown

test_s3_cluster/test.py:2130 - in test_graceful_shutdown
    assert errors == 0
E   assert 2 == 0

Unrelated to partition export (this PR doesn't touch test_s3_cluster). CI flagged it retry_failed (failed on first run and retry) and the node carries do_not_block_pipeline_on_failure: true — i.e. CI already treats it as a known-flaky, non-blocking test. → Pre-existing flaky. Safe to re-run.

Parser memory check — ERROR

Resolve master binary: No master `clickhouse-examples` artifact was found

Pure infra: the job compares the PR parser against the master baseline binary, and the master clickhouse-examples artifact was missing. Nothing in the diff can cause this. → Infra. Safe to re-run once the baseline artifact exists.

SQLLogic test — ERROR (Timeout)

errors: [{ "message": "Timeout", "from": "SQLLogic test" }]   # job ran 3h02m

Whole-job wall-clock timeout running the SQLite logic suites, not a test assertion failure — and SQLLogic exercises none of the code this PR changes. → Infra/timeout flake. Safe to re-run.

The aggregate PR check is red only because it surfaces the sub-results above (dominated by the non-blocking integration flake); it carries no independent failure.

PR health check

The green signal is strong. The port builds on every configuration (amd debug/asan_ubsan/binary/release, arm release, wasm_parser — all 0 failures), Fast test (11199 passed), all Stateless suites (debug parallel/sequential, distributed-plan s3, asan_ubsan selected — 0 failed), Unit tests (20695 passed), Stress tests, and AST/BuzzHouse fuzzers all pass. Most importantly, the regression suites that directly cover this feature — s3_export_part, s3_export_partition, iceberg_1/2, parquet, parquet_minio, parquet_aws_s3 — are all green, as are the new test_export_mt_partition_to_object_storage / test_export_partition_to_iceberg integration tests this PR adds. I would not block on any current red check; the remaining failures are branch-wide infra (Grype, swarms, Parser baseline), an external-dependency suite (oauth), and two re-runnable flakes (s3_cluster graceful-shutdown, SQLLogic timeout). No code change is needed from this PR to address them.

_{🤖 Static CI triage from praktika S3 reports + cross-PR comparison; I can't build/run ClickHouse, so correctness is ultimately validated by CI. Happy to dig into any specific check further.}

DimensionWieldr · 2026-10-15T01:28:54Z

Ran an AI audit which returned 2 high defects that codex already mentioned as P1 issues in the earlier review:

Preserve legacy Keeper keys during migration
and
Reject force replacement while the old task is committing

arthurpassos · 2026-10-15T11:56:51Z

Ran an AI audit which returned 2 high defects that codex already mentioned as P1 issues in the earlier review:

Preserve legacy Keeper keys during migration and Reject force replacement while the old task is committing

Both of these exist in the original PR. This is a port

DimensionWieldr · 2026-10-15T14:05:49Z

Ran an AI audit which returned 2 high defects that codex already mentioned as P1 issues in the earlier review:
Preserve legacy Keeper keys during migration and Reject force replacement while the old task is committing

Both of these exist in the original PR. This is a port

I see. In that case, I'll file them as a separate issue and ignore them for this PR.

DimensionWieldr · 2026-10-15T18:31:42Z

/iceberg/export partition/ice catalog/plain merge tree regression tests are passing.

Reran CI which cleared test_s3_cluster/test.py::test_graceful_shutdown. Results look fine.

LGTM.

arthurpassos added antalya port-antalya PRs to be ported to all new Antalya releases antalya-26.8 Session label (releasy session config) labels Oct 12, 2026

arthurpassos changed the title ~~Port plain MergeTree partition exports onto antalya-26.8~~ Antalya 26.8 - Forward port of #2290 - Plain merge tree partition export Oct 12, 2026

chatgpt-codex-connector Bot reviewed Oct 12, 2026

View reviewed changes

fix build

5705da6

This was referenced Oct 13, 2026

Antalya 26.8: Fix distributed global right join #2547

Merged

Antalya-26.8: Iceberg: support external paths in tables #2551

Merged

DimensionWieldr mentioned this pull request Oct 15, 2026

EXPORT PARTITION: force-replace and the new Keeper key can duplicate or strand exports #2576

Open

DimensionWieldr added verified Approved for release verified-with-issues Verified by QA and issues found. labels Oct 15, 2026

zvonand mentioned this pull request Oct 19, 2026

Port graph for antalya-26.8 #2322

Open

36 tasks done

arthurpassos merged commit de18994 into antalya-26.8 Oct 19, 2026
434 of 452 checks passed

zvonand added the forwardport This is a frontport of code that existed in previous Antalya versions label Oct 20, 2026

Conversation

arthurpassos commented Oct 12, 2026

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Documentation entry for user-facing changes

CI/CD Options

Exclude tests:

Regression jobs to run:

Uh oh!

chatgpt-codex-connector Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

github-actions Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

DimensionWieldr commented Oct 14, 2026

Uh oh!

blau-ai commented Oct 15, 2026

CI triage — @DimensionWieldr

Not PR-related

PR health check

Uh oh!

DimensionWieldr commented Oct 15, 2026

Uh oh!

arthurpassos commented Oct 15, 2026

Uh oh!

DimensionWieldr commented Oct 15, 2026

Uh oh!

DimensionWieldr commented Oct 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chatgpt-codex-connector Bot commented Oct 12, 2026 •

edited

Loading

github-actions Bot commented Oct 12, 2026 •

edited

Loading