Plain merge tree partition exports by arthurpassos · Pull Request #2290 · Altinity/ClickHouse

arthurpassos · 2026-08-28T17:30:25Z

List of changes:

Implements partition export for plain merge tree using Json task descriptors stored on disk, the structure is simplified because it is much simpler than the replicated case
Introduces the system.partition_exports table that keeps track of both plain and replicated cases. The old system.replicated_partition_exports is aliased to this new one for backwards compatibility.
Escapes each portion of the partition export key - this is not backwards compatible.
Stop shipping empty parquet files on plain object storage for exports with zero rows
Throws on a few errors zk that were left as todo
Refactor the test suite so that it is parametrized for both engines instead of duplicating the tests

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:

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:

github-actions · 2026-08-28T17:31:49Z

Workflow [PR], commit [2fca479]

arthurpassos · 2026-08-29T17:14:51Z

@codex review

chatgpt-codex-connector · 2026-08-29T17:15:05Z

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-12T14:11:12.740611Z	`2662683`	Manual request

ℹ️ 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.

arthurpassos · 2026-08-29T17:20:28Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 240da8173c

ℹ️ 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-08-29T17:37:02Z

+        catch (...)
+        {
+            replicated_error = std::current_exception();
+            tryLogCurrentException(getLogger("InterpreterKillQueryQuery"),
+                "KILL EXPORT PARTITION: could not read system.replicated_partition_exports (the WHERE may "
+                "reference columns that only exist for plain MergeTree); ignoring ReplicatedMergeTree tables");


Propagate system-table read failures during KILL

If either system-table query fails for an operational reason such as a memory limit, timeout, or internal exception, this broad catch treats it like an engine-specific unknown column and continues with the other table. The command can therefore report successful results while silently leaving matching exports from the failed table running. Only suppress the specific predicate-resolution error that proves the filter belongs exclusively to the other table; propagate all other failures.

AGENTS.md reference: AGENTS.md:L153-L153

Useful? React with 👍 / 👎.

…ya-26.6/export-partition-plain-merge-tree

arthurpassos · 2026-09-04T11:53:25Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8d038d350

ℹ️ 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".

arthurpassos · 2026-09-11T11:52:25Z

 ### Commit info columns

-These columns surface paths produced by the destination storage during commit, so it is possible to inspect what was written without consulting the destination directly:
+These columns surface paths produced by the destination storage during commit, so it is possible to inspect what was written without consulting the destination directly. They are populated for `Replicated*MergeTree` sources only; a plain `MergeTree` does not persist the commit paths, so they stay empty there even after a successful commit.


This shall be implemented for plain merge tree as well

Is this going to be covered in another PR?

I plan to cover it in this pr unless it is too complex (I doubt it is).

For now I am reviewing my own code (AI code :D)

DimensionWieldr · 2026-09-11T17:11:57Z

Iceberg export partition regression suite is fully passing on no/ice/glue catalogs with plain merge tree tables.

I will wait for dev review and the CI run to do a final CI triage to confirm no PR caused failures.

mkmkme

Overall looks good and worth an approval. I left some comments, I'll let you decide what to do about them

mkmkme · 2026-09-12T09:33:23Z

+The partition export tasks of both engines can be observed through `system.partition_exports`.

-The export task can be killed by issuing the kill command: `KILL EXPORT PARTITION <where predicate for system.replicated_partition_exports>`.
+`system.replicated_partition_exports` is kept as an alias of `system.partition_exports` for backwards compatibility. It returns exactly the same rows, including exports of plain `MergeTree` tables. Filter on `source_table` (or join against `system.tables`) if you need only one engine.


That's a bit counter-intuitive IMHO. But I don't have a good and simple solution for that. I can suggest one of two things:

Make replicated_partition_exports only return exports from RMT (but it will likely require quite a bit of work for not exactly a great result)

Mark system.replicated_partition_exports explicitly as deprecated (but I am not sure we have such a mechanism for system tables)

I don't think users will want to separate replicated from plain, this is a separation that naturally popped up simply because of the order these features were implemented.

On the deprecation approach, here is what AI found:

There is no first-class “deprecated system table” mechanism in ClickHouse. Settings and functions have one; system tables do not.
What exists today is the same pattern this branch already uses: attach the same storage under a second name and put the message in the table comment.

mkmkme · 2026-09-12T10:04:44Z

 ### Exception columns

- `last_exception_per_replica` is an `Array(Tuple(replica String, message String, part String, time DateTime, count UInt64))`. Each tuple is the most recent exception observed by a single replica plus a best-effort within-replica `count`. Replicas that have never reported an exception are omitted.
+- `last_exception_per_replica` is an `Array(Tuple(replica String, message String, part String, time DateTime, count UInt64))`. Each tuple is the most recent exception observed by a single replica plus a best-effort within-replica `count`. Replicas that have never reported an exception are omitted. A plain `MergeTree` export runs on a single node, so it contributes at most one tuple and its `replica` is empty.


Haven't found (yet) in the documentation, but wanted to ask: does it make sense for EXPORT PARTITION on a plain MergeTree to be synchronous? And instead of finding the exception in the system table, it would raise the exception as a response?

Nope, it shall be async. A part might take a day to be exported.

mkmkme · 2026-09-12T10:34:45Z

+        if (!iceberg_metadata)
+            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Destination storage {} is a data lake but not an iceberg table", dest_storage->getName());
+
+        if (!context->getSettingsRef()[Setting::allow_insert_into_iceberg])


Nitpick: I would probably move this check before checking for iceberg_metadata being nullptr, but it's not a biggie

mkmkme · 2026-09-12T11:12:15Z

+        auto & entry = it->second;
+
+        if (entry.getDescriptor().status != MergeTreePartitionExportTask::Status::PENDING)
+            return CancellationCode::CancelCannotBeSent;


I'd say this also deserves a LOG_INFO

mkmkme · 2026-09-12T11:49:33Z

+        std::lock_guard lock(mutex);
+        auto it = findByTransactionId(transaction_id);
+        if (it == tasks.end())
+            return;


Maybe a LOG_DEBUG here? Why can this happen?

mkmkme · 2026-09-12T12:39:33Z

    expected_status="COMPLETED",
    timeout=60,
    poll_interval=0.5,
+    system_table="partition_exports",


Is this argument (here and below) is actually used in a way that replicated_partition_exports is passed explicitly?

Nope, leftover from previous implementations. Fixed in 83e3f87

mkmkme · 2026-09-12T13:54:19Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2662683255

ℹ️ 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-09-12T14:11:10Z

+        return ExportPartitionUtils::compositeKey(
+            manifest.partition_id, manifest.destination_database, manifest.destination_table);


Preserve or migrate the previous ZooKeeper task keys

After upgrading with an existing replicated export, its ZooKeeper child still uses the old <partition>_<database>.<table> key, while this method now derives the escaped dot-separated key. Although poll reads the old child, the inserted entry is indexed by this new derived key and removeStaleEntries immediately removes it because that key is absent from the ZooKeeper child list; the scheduler therefore cannot resume its remaining parts, and a new export also misses the old path during duplicate detection. Migrate existing nodes or retain the actual ZooKeeper child key in the entry.

Useful? React with 👍 / 👎.

Discussed in #2290 (comment)

chatgpt-codex-connector · 2026-09-12T14:11:10Z

+        entry.part_references = std::move(part_references);
+        entry.setDescriptor(std::move(descriptor));
+        persist(composite_key, entry.getDescriptor().toJsonString());
+        tasks.insert_or_assign(composite_key, std::move(entry));


Block force replacement while the prior commit is running

When a forced re-export arrives while the existing entry has committing == true, this replacement proceeds even though KILL explicitly refuses that state. killExportPart cannot cancel the destination commit, so the old commit can publish successfully and then find its registry entry replaced, while the new transaction subsequently exports and commits the same partition again; for an Iceberg destination this can produce duplicate rows under two transaction IDs. Reject or defer the replacement until the existing commit lease is released.

Useful? React with 👍 / 👎.

Discussed in #2290 (comment)

arthurpassos · 2026-09-14T12:56:19Z

@mkmkme shared with me a couple of AI findings, below you can find the assessment of each:

Pre-change RMT export znodes are invisible to duplicate protection

True, but the feature is still marked as experimental. Adding a conversion layer adds complexity, I think it is better to make this a breaking change.

Malformed descriptor crashes ATTACH / startup

This is complaining about blocks like the below, but the json object is a Poco::Ptr, which has a null check:

if (json->has("last_exception"))
        {
            const auto exception_object = json->getObject("last_exception");
            task.last_exception.message = exception_object->getValue<String>("message");

loadFromDisk still omits unreadable PENDING work

Honestly, I think that's the only sane way of handling this. If we can't read it, what can we do about it aside from logging?

Missing source part stays retryable on the plain path

Fixed on e21bc07

force bypasses the committing lease

Force operation is not safe by design

Persist-then-apply drops in-memory success when the local write throws

Presented two "issues".

The first one is about killing an already completed export because the write to disk has ffailed. It doesn't really have a fix afaict, it is a split brain issue. The case is very narrow.

The second one is only applicable to plain exports and is about file already exists policy == error after a re-start with the commit file already present. Before this PR, it would throw and kill the export, even though it has been committed to the destination. This PR now changes it so that it considers it a success without reading any further data. Fixed in f0422be

Huge task_timeout_seconds can immediate-KILLED

Fixed by 67bbe0e

Last-part completion holds export_manifests_mutex across dest commit

This is true, but I would rather not optimize locking and threading until we face an actual performance issue.

…estination

…n-plain-merge-tree

arthurpassos added 5 commits August 28, 2026 14:02

1st vibe coded version

69f071a

some vibe coded fixes

3190603

persist-then-apply and uuidv4 instead of snowflakeid

d94ea53

idk

b2af06c

possibly fix build

f2ee03f

arthurpassos mentioned this pull request Aug 28, 2026

Plain merge tree partition export #2032

Closed

28 tasks

svb-alt added antalya antalya-26.6 labels Aug 28, 2026

chmod

240da81

arthurpassos marked this pull request as ready for review August 29, 2026 17:20

chatgpt-codex-connector Bot reviewed Aug 29, 2026

View reviewed changes

arthurpassos added 9 commits August 31, 2026 14:44

fix kill operation

9ff4a4a

fix backoff on plain

6c86d2f

make code a bit more readable, ai

a622f44

release parts

23d90fa

fix endless commit due to part ref lost

fe7e61a

well, I don't like it, but ok

7c959fb

fix dispatch err handling

f06a7dd

fix

7482d6b

Merge remote-tracking branch 'origin/antalya-26.6' into feature/antal…

c8d038d

…ya-26.6/export-partition-plain-merge-tree

chatgpt-codex-connector Bot reviewed Sep 4, 2026

View reviewed changes

arthurpassos added 2 commits September 4, 2026 09:15

throw exceptions to the user

875cc47

throw on unknown values

9496d85

arthurpassos changed the title ~~Feature/antalya 26.6/export partition plain merge tree~~ Plain merge tree partition exports Sep 4, 2026

simpĺify docs

3af519a

arthurpassos commented Sep 11, 2026

View reviewed changes

arthurpassos added 4 commits September 11, 2026 08:57

some documentation fixes

0433384

fix some more docs

b9f4406

improve naming

ff54cf5

more comments docs

7a8b440

arthurpassos added 5 commits September 11, 2026 14:34

rename method

968163b

documentation

8ef5a76

ai fixes

c70028c

persist commit info plain mt

2662683

simplify docs

e89554c

mkmkme self-requested a review September 12, 2026 07:36

mkmkme previously approved these changes Sep 12, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Sep 12, 2026

View reviewed changes

make no_such_data_part non retryable for plain exports

e21bc07

arthurpassos dismissed mkmkme’s stale review via e21bc07 September 14, 2026 12:24

arthurpassos added 11 commits September 14, 2026 11:20

fix issue of aborting export that has already been committed to the d…

f0422be

…estination

fix possible overflow, ai fixed

67bbe0e

Merge branch 'antalya-26.6' into feature/antalya-26.6/export-partitio…

9dc4c93

…n-plain-merge-tree

reorder setting check

182dd44

fix tests

83e3f87

add log

fa36968

rmv unneeded sentence

1eba19a

rename methdo

ffb5362

add more logs

66ecc47

add log

9c5845d

Merge branch 'antalya-26.6' into feature/antalya-26.6/export-partitio…

2fca479

…n-plain-merge-tree

CarlosFelipeOR mentioned this pull request Sep 14, 2026

CAS: object-store connections exhaust the container's ephemeral ports, fencing out the mount lease #2243

Open

		return ExportPartitionUtils::compositeKey(
		manifest.partition_id, manifest.destination_database, manifest.destination_table);

Uh oh!

Conversation

arthurpassos commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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

Uh oh!

Uh oh!

arthurpassos commented Aug 29, 2026

Uh oh!

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

Uh oh!

Codex Review Summary

Uh oh!

arthurpassos commented Aug 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

arthurpassos commented Sep 4, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DimensionWieldr commented Sep 11, 2026

Uh oh!

mkmkme left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkmkme commented Sep 12, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

arthurpassos commented Aug 28, 2026 •

edited

Loading

github-actions Bot commented Aug 28, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Aug 29, 2026 •

edited

Loading

arthurpassos commented Sep 14, 2026 •

edited

Loading