Guard rollback-sensitive transaction flags - #8245
Open
Amaury Chamayou (achamayou) wants to merge 1 commit into
Open
Guard rollback-sensitive transaction flags#8245Amaury Chamayou (achamayou) wants to merge 1 commit into
Amaury Chamayou (achamayou) wants to merge 1 commit into
Conversation
Amaury Chamayou (achamayou)
force-pushed
the
achamayou-guard-tx-flags
branch
from
August 31, 2026 07:51
4c22cdd to
2cbb194
Compare
This was referenced Aug 31, 2026
A transaction may request a forced ledger chunk, or arm a snapshot at the next signature. Both were applied after the transaction's writes, with no recheck that a concurrent view change had not already discarded them, so a transaction that never reached the ledger could still leave a chunk boundary behind, or arm a snapshot. Apply both under version_lock via Store::apply_tx_flags, refusing when the transaction's view or rollback epoch no longer holds, and attach the forced chunk to the transaction's own version rather than whichever version the store had since reached. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 75d99c5d-6efa-4048-8032-8c78b97208d9
Amaury Chamayou (achamayou)
force-pushed
the
achamayou-guard-tx-flags
branch
from
August 31, 2026 12:29
2cbb194 to
03bb4a1
Compare
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
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.
Fourth of a stack of small fixes for a family of key-value store races found by an interleaving-exploration harness. Stacked on #8244.
The problem
A transaction can request two side effects that outlive it:
CommittableTx::commit()applied both immediately after allocating a version and applying its writes, beforeStore::commit()decides whether the transaction can be replicated at all. Neither application rechecked the transaction's view or rollback epoch, so if a concurrent election rolled the store back in that window:SNAPSHOT_AT_NEXT_SIGNATUREwas re-armed on the store afterStore::rollback()had deliberately cleared it, so the node takes a snapshot it was told not to take.Separately,
force_end_of_chunkwas called withversionread back off the transaction after the fact. That is the transaction's own version today, but it is trivially fragile: any caller reaching this path with a stale handle attaches the boundary to the wrong entry.The fix
Both effects move behind a single new
AbstractStore::apply_tx_flags, which takesversion_lockonce and applies them only if the transaction's view and rollback epoch still hold. On failure the transaction returnsFAIL_NO_REPLICATE, which is whatStore::commit()would have returned a moment later anyway - the transaction did not reach the ledger, so it must not leave anything behind.The read-only (
NoVersion) case is preserved explicitly: such a transaction has no version to attach a chunk to, but arming a snapshot is still legal and unchanged from before.Why this is minimal
version_lock, taken exactly whereset_flagalready took it. Lock order (commit_lock->version_lock->chunker_lock) is unchanged.Test
Rollback-sensitive transaction flags are not restoredinkv_testcallsStore::apply_tx_flagsdirectly - single-threaded, no harness - and asserts:Mutation-verified: disabling the guard fails (1); using the store's
versioninstead of the transaction's fails (3).Labelled
run-long-test.Review stack
These five PRs come from one investigation and are stacked; review and merge in order.
All were found by an interleaving-exploration harness built over the real KV, consensus and history stack (draft #8238). The harness itself is deliberately not included here; these PRs carry only the fixes and the single-threaded regression tests that pin them.