Repository navigation
fix(storage): keep prepared transactions' WAL records until they commit or roll back (#188) - #246
Closed
CaffeinatedCoder wants to merge 1 commit into
Closed
CaffeinatedCoder wants to merge 1 commit into
CaffeinatedCoder wants to merge 1 commit into
Conversation
…it or roll back (#188) - PrepareTransactionAsync appends its Begin/Write records and flushes under _commitLock, like a commit, so they no longer interleave with a group-commit batch and no truncation runs while they are written. Returns false if the lock cannot be taken within WriteTimeoutMs. - A prepared transaction is tracked until it commits or rolls back (CompleteTransaction). Meanwhile checkpoints still write pages but skip the WAL truncation, and RecoverAsync skips it too, so MarkTransactionCommittedAsync, which appends only the commit record, finds the prepared Write records in the WAL and the commit survives a crash. - Document the behaviour, including that an unresolved prepare holds back truncation and that recovery rolls back a transaction still prepared. - Drop comments that said prepare writes without _commitLock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CaffeinatedCoder
had a problem deploying
to
Production
October 16, 2026 20:59 — with
GitHub Actions
Failure
Closed
5 of 72 tasks
This was referenced Oct 17, 2026
Contributor
|
Not merging. The loss in #188 needs a caller to drive MarkTransactionCommittedAsync directly, which BLite never does (#188 is low/P3). The PR adds new behaviour to checkpoint and recovery: prepare can now fail on commit-lock contention, and an unresolved or failed prepared transaction blocks WAL truncation until the engine closes. Too much risk for that path; a change confined to MarkTransactionCommittedAsync, or documenting/obsoleting the direct 2PC entry points, would be proportionate. |
This branch was successfully deployed
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.
Fixes #188 (single-process part; the multi-process part was closed by #201, which rejects these paths with
AllowMultiProcessAccess).MarkTransactionCommittedAsyncappends only a commit record and relies on the Begin/Write recordsPrepareTransactionAsyncwrote. Prepare held no_commitLock, and a prepared transaction's pages sit in_walCache, so a checkpoint (orRecoverAsync) could truncate the WAL between prepare and mark. The commit then had no Write records: after a crash before the next checkpoint, recovery replayed nothing and the committed pages were lost. (The in-libraryPrepareAsync→CommitAsyncflow rewrites everything through group commit and was not affected.)Changes:
PrepareTransactionAsync(both overloads) appends and flushes under_commitLock, like a commit, so its records no longer interleave with a commit batch, and it registers the transaction under that lock. If the lock is not free withinWriteTimeoutMsit returnsfalse, as for any other failure. Under heavy commit load a prepare can therefore fail where it used to write unsynchronised.CompleteTransaction), checkpoints still write pages but skip the WAL truncation;RecoverAsyncskips it too and records the written pages in_checkpointedWalPages, as a non-truncating checkpoint does.Transaction.Disposeand engineDisposeroll back what they own), and recovery applies only committed transactions, so a transaction still prepared at close or crash is rolled back on reopen.Not changed:
_walCacheis in memory, transaction ids restart at 1), so a prepare that survives a participant crash would be a new feature.CommitTransactionAsync(ulong)was already self-contained under_commitLock.CommitTransactionAsync(Transaction)fails is removed from_activeTransactions, so engineDisposeno longer rolls it back; it then holds back truncation until the engine closes. Documented, no counter/warning added.MarkTransactionCommittedAsyncstill takes_commitLockwith a blockingWait()without timeout (pre-existing).Tests
PreparedTransactionWalTests(single-process; engine disposed without checkpoint, then reopened):MarkTransactionCommittedAsync, reopen: page recovered (lost on main)RecoverAsync, mark, reopen: page recovered (lost on main)tx.PrepareAsync()keeps the WAL across a checkpoint; afterCommitAsync/RollbackAsyncthe next checkpoint truncates again, and the rolled-back page is not appliedAll 5 fail on main. Full BLite.Tests suite passes.
Conflicts with other open PRs
Conflicts with #310 (write-write conflict detection, #304) in
src/BLite.Core/Storage/StorageEngine.Transactions.cs. Resolution:CompleteTransactionremoves the transaction from both_preparedTransactions(here) and_baseVersions(#310).PrepareCoreAsynccalls #310'sThrowIfConflictingunder the commit lock, before the Begin record, so a conflicting prepare returns false and writes nothing.MarkTransactionCommittedAsynckeeps #310's check, and its prepared records then never get a commit record. Two tests inPreparedTransactionWalTestscover this. Either PR can land first; I will push the resolution to the second.🤖 Generated with Claude Code