Repository navigation
TTL Export and refactor of partition export into distributed part export - #2442
arthurpassos wants to merge 18 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Refuse a replacement destination with a different UUID
When a plain MergeTree export remains pending while its destination is dropped and recreated under the same name, this reconstructs the destination from its name alone even though the persisted descriptor records destination_uuid. The task can therefore write and commit into the replacement table; for an EXPORT TTL, the UUID-keyed scheduler may then export the same ranges again under the new destination key, producing duplicate rows. Resolve or validate the recorded UUID before dispatch and commit, and fail the task when the named table has a different identity.
AGENTS.md reference: AGENTS.md:L153-L153
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Honor the persisted destination UUID when resuming exports
For a plain MergeTree export, the descriptor persists destination_uuid, but this reconstructs the destination using only its database and table name, and the commit path performs the same name-only lookup. If the destination is dropped and recreated under the same name while a task is pending or while the server is down, the old task can write and commit its files into the replacement table instead of failing closed. Resolve or validate the destination against descriptor_copy.destination_uuid before exporting and committing.
ℹ️ 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".
| if (part->rows_count == 0) | ||
| continue; |
There was a problem hiding this comment.
When an active zero-row part is retained, for example with remove_empty_parts = 0, this branch leaves its range permanently in the NONE state. After another part in the partition is exported, ExportFence::checkCanMerge rejects merging the empty part with that neighbor because their export states differ, so even OPTIMIZE cannot consolidate the partition; a due delete TTL can also keep reporting the empty part as held. Record zero-row ranges as exported or otherwise exempt them from the fence instead of skipping them indefinitely.
Useful? React with 👍 / 👎.
arthurpassos
changed the title
- `MergeTreeData::exportParts` starts an export task and updates the export index atomically for both engines, and `exportPartitionToTable` is shared by `MergeTree` and `ReplicatedMergeTree`. - One `ExportTTLScheduler` class over an `IExportTTLIndex` (`MergeTreeExportTTLIndex` or `ReplicatedExportTTLIndex`) replaces the per-engine schedulers. - Every check ships the due parts of each partition as one group, with at most one group in flight per partition, so `ttl_export_check_period_seconds` (now 60 by default) is the batch interval. The batch settings and the `first_eligible_time` and `next_group_time` columns of `system.ttl_exports` are removed; `system.ttl_exports` is computed when it is queried. - `system.ttl_exports` reads nothing from Keeper: every replica of a table with an `EXPORT` TTL keeps its copy of the export index and of the scheduler replica up to date with watches on `version` and `scheduler_lock`. A table without one takes no scheduler lock and arms no watches. - A failed task is retried with exactly its claimed parts under the same `commit_id`, which replaces `retry_of`, so the destination commits them once even if the failed task landed after all. - A part without rows, e.g. emptied by a mutation, is exported as no files by `EXPORT PART`, `EXPORT PARTITION` and the TTL alike, without reading its min/max index, which is not initialized.
… this allows us to keep merging exported parts
…ya-26.6/export-partition-iterations # Conflicts: # tests/integration/test_export_merge_tree_part_to_iceberg/test.py
This branch has not been deployed
2 participants
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: