Skip to content

Stable 24.8 Backport of #99628 - Fix TOCTOU race in vertical merge cancellation - #2816

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

Stable 24.8 Backport of #99628 - Fix TOCTOU race in vertical merge cancellation#2816
blau-ai wants to merge 2 commits into
releases/24.8.14from
backports/releases/24.8.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
21ff418 
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
099032e 
Took the PR's latch-cancellation code in the horizontal executeImpl conflict; the vertical-stage hunk applied cleanly.
@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:

  • 21ff418: cherry-pick with the conflicts committed as-is

  • 099032e: resolution

  • Conflict in src/Storages/MergeTree/MergeTask.cpp, in ExecuteAndFinalizeHorizontalPart::executeImpl. Release 24.8.14 had the older plain if (ctx->is_cancelled() || ...pull(block)), and the PR's merge had already brought in the if (cancelled) ...store(true) lines in the block below it.

  • Resolved by taking the PR side: bool cancelled = ctx->is_cancelled(); is read once, then if (cancelled || !pull(block)) decides whether to finalize.

  • The vertical-stage hunk in executeVerticalMergeForOneColumn applied cleanly and matches the PR.

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

24.8 24.8.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