Skip to content

Remove lock inside the task export partition#1713

Open
arthurpassos wants to merge 1 commit intoantalya-26.1from
export_partition_remove_lock_in_task
Open

Remove lock inside the task export partition#1713
arthurpassos wants to merge 1 commit intoantalya-26.1from
export_partition_remove_lock_in_task

Conversation

@arthurpassos
Copy link
Copy Markdown
Collaborator

Removing export_merge_tree_partition_lock_inside_the_task because 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):

  • Improvement

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

Remove the export_merge_tree_partition_lock_inside_the_task setting. No longer maintained

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

@arthurpassos arthurpassos added antalya port-antalya PRs to be ported to all new Antalya releases labels Apr 30, 2026
@github-actions
Copy link
Copy Markdown

Workflow [PR], commit [3588da0]

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya port-antalya PRs to be ported to all new Antalya releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant