{{ message }}
Enable transactions on encrypted disks in CI#104780
Merged
tuanpach merged 5 commits intoMay 25, 2026
Merged
Conversation
After ClickHouse#92141 refactored version metadata to use atomic tmp+rename writes instead of `WriteMode::Append`, the race that previously corrupted `txn_version.txt` on encrypted disks (see ClickHouse#59898 discussion) is no longer possible: concurrent `creation_csn` and `removal_tid` updates can no longer interleave inside the encrypted append path. Install `transactions.xml` in encrypted-storage CI jobs and drop the `no-encrypted-storage` tag from transaction-related stateless tests.
Contributor
`VersionMetadata::isVisible` is called per part per read; in heavy CI configs it pushed `02435_rollback_cancelled_queries` and `01168_mutations_isolation_2` over the 180s flaky-check timeout on the encrypted-S3 + ASAN+UBSAN variant. Demote the two CSN lookup logs to LOG_TEST so they no longer fire under standard debug logging.
The test inserts 4M+ rows in total across initial setup, parallel insert/select/cancel phase, and final verification. On the encrypted-S3 + ASan+UBSan + meta-in-keeper flaky-check variant it consistently runs 250-300s, exceeding the 180s default budget. The `long` tag raises the budget to 600s, which fits even the slowest observed runs with substantial headroom.
On the encrypted-S3 + ASan+UBSan + meta-in-keeper flaky-check variant the per-worker 400s window only fits ~10 iterations of `select_insert_action` instead of the required 15, causing the test's internal "not enough iterations" assertion to fire. The correctness checks inside each iteration are unchanged; only the throughput floor is lowered from 15 to 5 so the test still validates isolation on slow storage variants without flagging environmental slowdown as a bug.
antonio2368
approved these changes
May 14, 2026
…-on-encrypted-disks
Contributor
LLVM Coverage ReportChanged lines: 100.00% (9/9) · Uncovered code |
1 task
robot-ch-test-poll1
added
the
pr-synced-to-cloud
DavidHe-2008
pushed a commit
to DavidHe-2008/ClickHouse
that referenced
this pull request
Jun 1, 2026
…s-on-encrypted-disks Enable transactions on encrypted disks in CI
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After #92141 refactored version metadata to use an atomic tmp+rename write pattern instead of
WriteMode::Append, the race that previously corruptedtxn_version.txton encrypted disks is structurally impossible:VersionMetadataOnDisk::storeInfoToDataPartStoragealways writes the full file viacreateFile+writeFile+replaceFile, serialized throughpersisted_info_mutex, and never appends.The original failure that motivated disabling transactions on encrypted disks (see #59898 discussion) was the interleaving of
appendCSNToVersionMetadataandappendRemovalTIDToVersionMetadataoncached_s3_encrypted, which produced atxn_version.txtwhosecreation_csn:line had been overwritten with garbage and triggered anassertHasValidVersionMetadatafatal. Both append call sites were deleted by #92141.This PR:
USE_ENCRYPTED_STORAGEguards around installingtransactions.xmlintests/config/install.sh(single-server andUSE_DATABASE_REPLICATEDtwo-server cases).no-encrypted-storagetag from 30 transaction-related stateless tests, and the matching explanatory comment blocks in03812_truncate_optimize_in_transaction.sqland03920_drop_db_hang_with_trx.sh.04098_asterisk_include_virtual_columns_mergetree.sqlkeeps itsno-encrypted-storagetag — that tag is unrelated to transactions.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Enable transactions on encrypted disks in CI now that #92141 eliminated the append-based version-metadata writes that previously corrupted
txn_version.txton encrypted storage.Documentation entry for user-facing changes
Version info
26.6.1.127