Skip to content

fix(txn): log an ABORT record when a committed transaction cannot be applied - #1380

Draft
zahariash wants to merge 1 commit into
LadybugDB:mainfrom
zahariash:fix/commit-applied-after-failed-publish
Draft

zahariash wants to merge 1 commit into
LadybugDB:mainfrom
zahariash:fix/commit-applied-after-failed-publish

Conversation

@zahariash

Copy link
Copy Markdown
Contributor

This work was produced with the help of language models.

Fixes #1377.

A COMMIT that failed with Buffer manager exception: Unable to allocate memory! The buffer pool is full and no memory could be freed! after its WAL commit record was durable was rolled back in memory, but a crash before the next checkpoint made recovery apply it. #1377's repro reopened with 187 nodes where the connection saw 186, and with the failed primary-key insert retried, reopening failed with Found duplicated primary key value 186, which violates the uniqueness constraint of the primary key column.. A clean close or a checkpoint before the crash dropped the transaction instead.

Fix. When applying a transaction fails after its COMMIT record is durable, TransactionManager::commit appends and syncs an ABORT record that names the transaction by the WAL offset where its records start, then reports the original error. Recovery's dry pass collects the aborted transactions, and the replay loops of the active WAL, the frozen WAL and graph WALs skip their records. A failed COMMIT is therefore never applied, and the database stays writable. If the ABORT record cannot be written either, the WAL refuses writes until a reopen, which then applies the transaction; the error says so. A transaction's records are appended as one block, and a checkpoint waits until it is rolled back, so its COMMIT and ABORT land in the same file even with several writers. A crash between the two leaves the transaction applied, as a crash during any COMMIT may. The unused Transaction::commit(WAL*), a second publish path without this handling, is removed, and tools/wal_dump prints the new record.

Compatibility. ABORT is a new WAL record type. An older build treats it as a corrupt tail: it stops replay there and applies the transaction it undoes, or refuses to open with throwOnWalReplayFailure. The record lasts only until the next checkpoint retires the WAL; docs/checkpoint_recovery.md describes it and lists it with the other records older builds cannot read.

Tests. In FailedCommitTest (test/transaction/transaction_test.cpp), FailedCommitIsNotAppliedAfterCrash, RetriedPrimaryKeyInsertReopensAfterCrash and FailedCommitOfTwoTablesLeavesNeither crash after a COMMIT that fails while being applied; main reopens with the failed relationship, fails on the duplicated key, and keeps the failed node. AbortRecordSkipsOnlyTheNamedTransaction, AbortRecordSkipsTheTransactionInAFrozenWAL, TornAbortRecordLeavesTheTransaction and AbortRecordNamingNoTransactionIsIgnored append an ABORT to a WAL: naming the middle one of three transactions, in a WAL renamed to the frozen WAL, torn, and naming no transaction. FailedAbortLeavesTheTransactionToRecovery makes the ABORT fail too, and WalTest.AbortSyncFailurePoisonsWAL fails its sync. The skip in graph-WAL replay has no test, since it needs a failing commit in a standalone session on the graph's file. KeepsCheckpointedRels (#1195) and ConcurrentDropGraphDuringStagedInsertCommitFailsCleanly pass unchanged. Full ctest: 3190/3190; storage-compat suite, every release: same results as main.

Touches the same code as #1378 (WALReplayer::dryReplay and the replay loops); merging the two keeps both changes, and that merge is what the stress run in #1376 used.

…applied

TransactionManager::commit makes a transaction's COMMIT record durable before
publishCommit() applies it. When publish failed (a full buffer pool, or a named
graph dropped by a concurrent DROP GRAPH), the caller rolled the transaction
back in memory while the WAL kept the record, so a crash before the next
checkpoint made recovery apply a COMMIT the client saw fail; a retried
primary-key insert then left a database that no longer opened.

- After such a failure, append and sync an ABORT record (new WAL record type)
  naming the transaction by the WAL offset where its records start, then
  report the original error. If the ABORT cannot be written either, poison
  the WAL: writes are refused until a reopen, which applies the transaction.
- Recovery's dry pass collects aborted transactions; the replay loops of the
  active, frozen and graph WALs skip their records.
- Remove the unused Transaction::commit(WAL*), a second publish path without
  this handling; tools/wal_dump prints the new record; docs describe it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: a COMMIT that fails because the buffer pool is full is applied after a crash

1 participant