fix(2pc): clean up prepared transactions recorded in the WAL after a restart - #1271
Open
willothy wants to merge 21 commits into
Open
fix(2pc): clean up prepared transactions recorded in the WAL after a restart#1271willothy wants to merge 21 commits into
willothy wants to merge 21 commits into
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
willothy
force-pushed
the
fix/2pc-wal-recovery-gid-prefix
branch
from
July 27, 2026 20:58
09c864c to
09beb8a
Compare
willothy
marked this pull request as ready for review
July 27, 2026 21:01
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.
Problem
With the 2pc WAL enabled, transactions in flight during a SIGTERM/SIGKILL were left as orphaned prepared transactions on the shards after PgDog rebooted, even though WAL recovery ran.
Since #1248, prepared transaction GIDs embed
instance_id(), which (withoutNODE_ID) is a random string generated per process. The WAL only persisted the numeric transaction ID, so after a restart, cleanup re-rendered the GID with the new process's instance ID and issuedROLLBACK PREPARED/COMMIT PREPAREDagainst a name that doesn't exist. The 42704 ("prepared transaction does not exist") skip intwo_pc_on_guardsmade that look like success, so cleanup wrote End records to the WAL and permanently hid the orphans. Undecided transactions kept holding locks. Decided ones were left half-committed.Solution
Record the coordinator GID prefix in every WAL Begin record (and checkpoint entry), so recovery knows the exact prepared transaction names on the shards. The field is additive (
#[serde(default)]); no tag or segment format changes.Make
Binding::two_pc_cleanupscanpg_prepared_xactson each shard (filtered tocurrent_database()) and resolve the transaction by its exact recorded GID. The scan is what makes retries idempotent: a resolved GID stops appearing, so nothing is re-committed or re-rolled-back. A GID that shares the numeric ID under a different prefix belongs to another coordinator and is never touched. For WAL records written by versions that did not store the prefix, matching falls back to the durable numeric transaction ID plus shard index; a wrong-target action through the fallback requires a 64-bit random ID collision between concurrently active transactions.Manager::cleanup_phaseuses this for all cleanup (WAL recovery, interrupted clients, replication copy).Supporting hardening:
TwoPcTransaction::from_strnow requires the__pgdog_2pc_prefix, so cleanup can never touch another application's prepared transactions.[A-Za-z0-9_-]; anything else is refused with a warning.NODE_IDandDEPLOYMENT_IDare validated against the same alphabet at startup (empty now behaves as unset, likeHOSTNAME). This makes the GIDs safe to embed in transaction control statements without escaping.COMMIT/ROLLBACK PREPAREDfailing with 42501 (insufficient privilege) warns with the exact statement for the operator instead of retrying forever and replaying on every restart.Testing
test_cleanup_transaction_foreign_prefix, fails onmain); numeric fallback for legacy records without the prefix field; and a structural-guarantee test proving cleanup leaves a same-number, different-prefix GID untouched.prefixkey) decodes with an empty prefix.integration/two_pc_crash_safetynow passes all 4 scenarios, including kill-mid-PREPARE followed by a restart with a fresh instance ID (this is the reported scenario, and it fails onmain). Its synthesized-WAL commit test writes records without the prefix field, covering the legacy fallback end to end. Itswal_helperno longer compiled (default-features = falsevspgdog-plugin'spg_queryrequirement); fixed here. Note this suite is not wired into CI orintegration/run.sh, which is how the regression shipped unnoticed; adding it to CI would be a good follow-up.Known limitations
__pgdog_2pc_*entries inpg_prepared_xacts.Closes #1175