Remove lock inside the task export partition#1713
Remove lock inside the task export partition#1713arthurpassos wants to merge 1 commit intoantalya-26.1from
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3588da0473
ℹ️ 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".
| json.set("ttl_seconds", ttl_seconds); | ||
| json.set("task_timeout_seconds", task_timeout_seconds); | ||
| json.set("lock_inside_the_task", lock_inside_the_task); | ||
| json.set("write_full_path_in_iceberg_metadata", write_full_path_in_iceberg_metadata); |
There was a problem hiding this comment.
Preserve
lock_inside_the_task in serialized manifests
Dropping lock_inside_the_task from ExportReplicatedMergeTreePartitionManifest::toJsonString() breaks rolling upgrades: a new replica now writes metadata without this key, but older replicas still parsing the same ZooKeeper metadata.json call getValue<bool>("lock_inside_the_task") and throw on missing field. In a mixed-version cluster, that makes export-partition tasks unreadable/failing on old nodes until every replica is upgraded. Keep writing the field (e.g., always false) for one compatibility window before removing it.
Useful? React with 👍 / 👎.
Removing
export_merge_tree_partition_lock_inside_the_taskbecause it was developed to benchmark against the default branch. In my benchmarks, it is no better. We don't seem to have zookeeper communication problems anymore, and it was not maitained. At this point, it does not even work properly, especially for iceberg.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Remove the
export_merge_tree_partition_lock_inside_the_tasksetting. No longer maintainedDocumentation entry for user-facing changes
...
CI/CD Options
Exclude tests:
Regression jobs to run: