Skip to content

Stable 25.3 Backport of #99628 - Fix TOCTOU race in vertical merge cancellation - #2817

Open
blau-ai wants to merge 2 commits into
releases/25.3.14from
backports/releases/25.3.14/99628
Open

Stable 25.3 Backport of #99628 - Fix TOCTOU race in vertical merge cancellation#2817
blau-ai wants to merge 2 commits into
releases/25.3.14from
backports/releases/25.3.14/99628

Conversation

@blau-ai

@blau-ai blau-ai commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Fix a rare logical error exception during vertical merge when SYSTEM STOP MERGES and SYSTEM START MERGES are executed concurrently. (ClickHouse#99628 by @Daedalus-Icarus)

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

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

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • CAS (content-addressed storage; Antalya only)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

Avogar and others added 2 commits November 9, 2026 11:05
@Avogar @zvonand
Merge pull request ClickHouse#99628 from Desel72/fix/issue-99578
486cbec 
Fix TOCTOU race in vertical merge cancellation

Conflicts committed as-is; the next commit resolves them:
- src/Storages/MergeTree/MergeTask.cpp (both modified)
@zvonand
Resolve conflicts in backport of ClickHouse#99628
168d5b0 
Kept the PR's latched `cancelled` variable in `executeImpl`, using the release branch's `ctx->is_cancelled()`.
@blau-ai

blau-ai commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

Port-AL: cherry-pick conflicts resolved by AI

Conflicted files:

  • src/Storages/MergeTree/MergeTask.cpp (both modified)

Commits:

  • 486cbec: cherry-pick with the conflicts committed as-is

  • 168d5b0: resolution

  • Conflict: in MergeTask::ExecuteAndFinalizeHorizontalPart::executeImpl, HEAD had the old single-line if (ctx->is_cancelled() || !...pull(block)). The cherry-picked change replaced it with a latched bool cancelled.

  • Resolution: I removed the conflict markers and the old line, and kept the PR's version. The cancellation is read once and stored into merge_list_element_ptr->is_cancelled, so finalize throws reliably.

  • Why it fits: the release branch's ctx->is_cancelled lambda already includes the blocker and the TTL blocker, so it works with the new code.

  • Vertical merge hunk: the executeVerticalMergeForOneColumn change applied without conflict, and I left it as is.

This branch has not been deployed

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

Reviewers

No reviews

Assignees

No one assigned

Labels

25.3 25.3.14 backport Backport stable

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@blau-ai @Avogar @zvonand