Retry transaction metadata writes under noexcept callbacks instead of terminating by filimonov · Pull Request #2396 · Altinity/ClickHouse

filimonov · 2026-09-17T07:15:34Z

MergeTreeTransaction::afterCommit and rollback are noexcept and write part version metadata and mutation CSNs to disk. A storage error in one of these writes escaped and terminated the server, although the transaction was already committed (or rolled back) in the transaction log and a restart repairs the files from it. This is what killed the server in the CAS test runs of the linked issue.

The six writes now go through one helper, retryMetadataStore: the error is retried with backoff for up to 60 seconds per object; LOGICAL_ERROR and NOT_IMPLEMENTED are rethrown at once; an exhausted budget, or a server shutdown, rethrows as before, so a lost write is never hidden. MergeTreeMutationEntry::writeCSN rewrites the whole mutation record through a temporary file, so a retry is idempotent. setMutationCSN logs a warning instead of throwing LOGICAL_ERROR when KILL MUTATION erased the entry during the commit window. Two ONCE failpoints and the stateless test 05053_transaction_metadata_store_retry cover the commit of parts, the commit of a mutation and a rollback.

The change is generic MergeTree code and is meant to be cherry-picked to upstream unchanged.

Closes: #2344

Changelog category (leave one):

Critical Bug Fix (crash, data loss, RBAC)

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

Fixed a server termination when a disk write of transaction metadata (part CSN or mutation CSN) failed inside the commit or rollback of a MergeTree transaction; such writes are now retried for a bounded time. Also fixed a termination when KILL MUTATION raced with the commit of a transactional mutation.

Documentation entry for user-facing changes

Documentation is not needed (no user-visible interface changes)

CI/CD Options

Exclude tests:

Regression jobs to run:

🤖 Generated with Claude Code

https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx

… terminating `MergeTreeTransaction::afterCommit` and `rollback` are `noexcept`. They write part version metadata (`creation_csn`, `removal_csn`, `removal_tid`) and the CSN of a mutation to disk. A storage error in one of these writes escaped and terminated the server, although the transaction was already committed (or rolled back) in the transaction log and a restart repairs the files from it. The six writes now go through `retryMetadataStore`: the error is retried with backoff for up to 60 seconds per object; `LOGICAL_ERROR` and `NOT_IMPLEMENTED` are rethrown at once; an exhausted budget, or a server shutdown, rethrows as before, so a lost write is never hidden. `MergeTreeMutationEntry::writeCSN` used to append one line to the mutation file; a write that fails half-way, or is repeated, could leave a partial or duplicated `csn` line, which the loader rejects. The whole record is now written through a temporary file and replaces the old one, so a retry is idempotent. `loadMutations` tolerates a temporary file that a repair earlier in the same pass has already consumed. `KILL MUTATION` between the log write and `afterCommit` erases the mutation entry and cannot roll the committing transaction back; `setMutationCSN` then threw `LOGICAL_ERROR` under `noexcept`. It now logs a warning: the parts are already mutated and there is nothing left to write. Two `ONCE` failpoints and the stateless test `05053_transaction_metadata_store_retry` cover the commit of parts, the commit of a mutation and a rollback. Closes: #2344 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>

github-actions · 2026-09-17T07:16:56Z

Workflow [PR], commit [dd44241]

…a-store-retry-squashed

- moved `[cas-txn-commit-inside-noexcept-aftercommit]` -> ref-protocol.md (KEEP; added a status note that the class fix is tracked as open PR Altinity#2396) - moved `[cas-transient-lease-fence-surfaces-to-clients]` -> ref-protocol.md (KEEP) Part of unit u1-inbox of the CAS docs grooming campaign (verdict: tmp/groom/u1-inbox/verdict.md, 36/36 APPLY). Original text carried over verbatim per the unit's applier-brief override. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…rs_config copy with async inserts disabled Profile settings are read by AccessControl from the users file, so command-line profile flags never reached them, and this tree defaults async_insert to 1 while async inserts are refused inside transactions. The recipe now copies users.xml, links the CI override disable_async_inserts.xml and points the server at the copy via users_config; the transaction check is BEGIN TRANSACTION; ROLLBACK and a fourth check asserts async_insert = 0. PR Altinity#2396 (in progress, CAS-177). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8mZSGiD8iJumpJMiQnrmC

…tation - `retryMetadataStore` takes a stopwatch started at the beginning of `afterCommit` / `rollback`, so the 60 s budget covers the whole callback instead of each object: a commit of N parts no longer waits up to N times the budget, and a retry on the unknown-state path blocks the log updating thread for at most one budget. - `killMutation` erases the map entry before deleting the file, so a retry after a failure could not delete the file anyway. It runs once; a non-invariant error is logged and the leftover file is removed at the next load. - `loadMutations` takes the directory listing before touching any file: repairing a record rewrites `mutation_N.txt` through a temporary file, and a directory iterator gives no guarantee about entries that change under it. - `writeCSN` sets the in-memory CSN after the file is replaced. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GhVd7eMAWdFubNk4g1B2Tx Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>

…-26.6/transaction-metadata-store-retry-squashed

alsugiliazova · 2026-10-01T09:46:49Z

PR #2396 Distributed-Systems Audit

AI audit note: This review was generated by AI (GPT-5.6 Sol).

Summary

No confirmed distributed-systems defects in the reviewed scope.

alsugiliazova · 2026-10-01T10:31:56Z

PR #2396 CI Verification Report

Verification (2026-10-01)

Field	Value
PR	Altinity/ClickHouse#2396 — Retry transaction metadata writes under noexcept callbacks instead of terminating
Base	`antalya-26.6`
Head	`dd44241da88f47239f2d45748ef261f962c0753e`
State	Open, approved, merge blocked by red checks
Run	36599597474
CI report	ci_run_report.html
Method	`pr-ci-failure-triage`, `upstream-test-investigation`, and `regression-test-database-investigation`; CI database rates plus job and server logs

Verdict

No current failure is caused by this PR, and no failure remains unknown. CI can be approved for PR causation, although GitHub is still red.

The final run has seven logically distinct failed jobs and one cancelled job. GitHub also shows duplicate commit-status contexts and the aggregate PR status.

Category	Jobs	Blocks #2396?
`regression`	0	—
`unknown`	0	—
`pre-existing-flaky`	4 — debug stress, CAS sanitizer stress, Swarms, CAS lightweight delete	No
`infrastructure`	3 — CAS selects, Grype server, Grype keeper	No
`cascade` / cancelled	1 cancelled CASS3Cache job; parent TestFlows paths and `Check failed` rows	No, but rerun the cancelled job

Current red checks

Check	Category	Evidence
Stress test (`amd_debug`)	`pre-existing-flaky`	AST fuzzing hit `Inconsistent KeyCondition behavior` in `MergeTreeDataSelectExecutor::markRangesFromPKRange`. The signature predates this PR; two of this PR's three debug-stress runs passed. The diff does not touch key analysis. Antalya 26.6 lacks upstream fixes #109023 and #111964.
Stress test (`amd_asan_ubsan`, CAS S3)	`pre-existing-flaky`	The live sanitizer server was still scanning S3 state when the startup-readiness deadline expired. The same startup timeout occurred in 21 of 75 unrelated runs and on identical commits. `Check failed` is a cascade row.
Swarms	`pre-existing-flaky`	After 1,017 scenarios passed, dynamic feature loading raised `TypeError: name must be specified`. History has 138 passes and 5 matching errors in 30 days. `/swarms` is the parent cascade of `/swarms/feature`.
CAS lightweight delete	`pre-existing-flaky`	Concurrent disjoint deletes left 507 rows instead of 500. The same family failed 19 of 443 runs in 30 days and predates this PR. No transaction-metadata retry error appeared.
CAS selects	`infrastructure`	The CAS backing lost its mount lease and rejected reads and mutations with `TRANSIENT unavailability, not damage`. The signature occurs across unrelated commits at a high baseline rate. Descendant scenario failures are cascades of the same outage.
Grype server and Grype keeper	`infrastructure`	Both images contain base-image `zlib 1.3.2`, newly flagged as High severity by CVE-2026-85091. The PR does not change image dependencies. The same scans fail on unrelated 26.6 PRs after the CVE publication.
CASS3Cache lightweight delete	`cascade` / cancelled	The suite step was cancelled after about five hours and uploaded no result rows or artifacts. It needs a rerun; it provides no evidence against this PR.

Direct coverage of the change

The added 05053_transaction_metadata_store_retry test passed on the final head in:

Fast test
amd_debug sequential
amd_asan_ubsan DB-disk sequential

The test is intentionally skipped on S3-storage lanes by its tags.

Earlier failures on the first two PR revisions are superseded on the final head:

Source upload: passed
Integration amd_asan_ubsan, db disk, old analyzer, 8/8: passed
Stateless amd_asan_ubsan, CAS S3, parallel, 1/2: passed
S3 export-part regression: passed

Recommendations

Rerun the four flaky jobs and the cancelled CASS3Cache job.
Treat the CAS mount-lease outage as test infrastructure unless a rerun reproduces a PR-specific transaction-metadata signature.
Update or waive the base-image zlib finding separately; rerunning Grype without an image change will remain red.
Backport upstream #109023 and #111964 separately to remove the debug KeyCondition stress crash.

filimonov added the CAS label Sep 17, 2026

Merge branch 'antalya-26.6' into fix/antalya-26.6/transaction-metadat…

86539d6

…a-store-retry-squashed

k-morozov self-requested a review September 22, 2026 17:30

svb-alt added antalya antalya-26.6 labels Sep 23, 2026

k-morozov requested changes Sep 24, 2026

View reviewed changes

Comment thread src/Storages/StorageMergeTree.cpp Outdated

Comment thread src/Interpreters/MergeTreeTransaction.cpp Outdated

Comment thread src/Interpreters/MergeTreeTransaction.cpp Outdated

Comment thread src/Storages/MergeTree/MergeTreeMutationEntry.cpp Outdated

filimonov mentioned this pull request Sep 27, 2026

CAS Backlog #2441

Open

68 tasks

filimonov and others added 2 commits September 29, 2026 17:30

Merge remote-tracking branch 'altinity/antalya-26.6' into fix/antalya…

dd44241

…-26.6/transaction-metadata-store-retry-squashed

filimonov requested a review from k-morozov September 29, 2026 17:00

k-morozov approved these changes Sep 30, 2026

View reviewed changes

alsugiliazova added the verified Approved for release label Oct 1, 2026

Navigation Menu

Retry transaction metadata writes under noexcept callbacks instead of terminating - #2396

Retry transaction metadata writes under noexcept callbacks instead of terminating#2396
filimonov wants to merge 4 commits into
antalya-26.6from
fix/antalya-26.6/transaction-metadata-store-retry-squashed

filimonov commented Sep 17, 2026 •

edited by alsugiliazova

Loading

Uh oh!

github-actions Bot commented Sep 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alsugiliazova commented Oct 1, 2026

Uh oh!

alsugiliazova commented Oct 1, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

filimonov commented Sep 17, 2026 • edited by alsugiliazova Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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:

Uh oh!

github-actions Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alsugiliazova commented Oct 1, 2026

PR #2396 Distributed-Systems Audit

Summary

Uh oh!

alsugiliazova commented Oct 1, 2026

PR #2396 CI Verification Report

Verification (2026-10-01)

Verdict

Current red checks

Direct coverage of the change

Recommendations

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

filimonov commented Sep 17, 2026 •

edited by alsugiliazova

Loading

github-actions Bot commented Sep 17, 2026 •

edited

Loading