v0.6.33: shmem term cache honours subtransaction aborts (#127) - #128
Merged
Conversation
…127) A term first interned inside a subtransaction that aborts — the PL/pgSQL EXCEPTION pattern, where the caught error rolls back only the subtransaction and the outer transaction commits — left its staged (fingerprint → dict_id) pair in the per-backend pending list, and the outer commit published it to the cross-backend shmem cache with the dictionary row already rolled back. Every later intern of that exact lexical value then resolved to the dangling id (get_term NULL, quads dereferencing to nothing) until pgrdf.shmem_reset() or a postmaster restart. One refused write inside an exception handler could poison shared vocabulary terms for every backend. Each staged entry now carries GetCurrentSubTransactionId() and an AbortSub callback drops entries at-or-above the aborting id — subtransaction ids are monotonic within a backend, so while a subtransaction is open every newer id is nested inside it, making >= the exact rollback set. Committed subtransactions and the outer transaction keep their entries; top-level commit/abort semantics are unchanged. Registration rides the existing once-per-transaction flag (pgrx clears subxact hooks at top-level transaction end). Three regression tests: abort discards, commit keeps, and an abort drops only its own entries (earlier top-level staging survives).
Version to 0.6.33 across Cargo.toml/lock, pgrdf.control, the compose per-file mount, the 0.6.32 -> 0.6.33 upgrade bridge (comment-only — the fix ships in the .so as behaviour, no DDL), and the CHANGELOG fold.
The smoke golden pins pgrdf.version()/extversion; the 0.6.33 bump missed it and CI regression caught exactly that (100 pass, 1 fail).
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.
Closes #127.
What
A term first interned inside a subtransaction that aborts — the PL/pgSQL
EXCEPTIONpattern, where the caught error rolls back only the subtransaction while the outer transaction commits — left its staged(fingerprint → dict_id)pair in the per-backend pending list, and the outer commit published it to the cross-backend shmem cache with the dictionary row already rolled back. Every later intern of that exact lexical value, from any backend, resolved to the dangling id (pgrdf.get_termNULL, quads dereferencing to nothing) untilpgrdf.shmem_reset()or a postmaster restart. One refused write inside an exception handler could poison shared vocabulary terms for every backend.Fix
Each staged entry now carries
GetCurrentSubTransactionId(), and anAbortSubcallback (pgrxregister_subxact_callback) discards entries at-or-above the aborting subtransaction's id. Subtransaction ids are monotonic within a backend and, while a subtransaction is open, every newer id is nested inside it — so>= my_subidis exactly the set whose dictionary INSERTs rolled back. Entries from committed subtransactions and the outer transaction survive; top-level commit/abort semantics are unchanged. Registration rides the existing once-per-transaction flag (pgrx clears subxact hooks at top-level transaction end).Tests
Three new
#[pg_test]s instorage::shmem_cache(the first was written before the fix and failed withleft: 1, right: 0— the staged entry surviving the abort):shmem_subxact_abort_discards_staged_entries— abort discardsshmem_subxact_commit_keeps_staged_entries— committed subtransactions keep their entries (no over-discard)shmem_subxact_abort_keeps_earlier_entries— an abort drops only its own entries; earlier top-level staging survivesFull suite: 384 passed / 0 failed (pg18, builder container). fmt + clippy
-D warningsclean.Proven end-to-end
On a disposable pg18 instance running 0.6.32, both repro layers confirmed the poison (issue #127 has the pure-SQL recipe: retry of an aborted-subxact intern returned an id with 0 dictionary rows; a validation-refused write followed by an identical retry failed on values provably present). After upgrading that instance to this branch's build (
ALTER EXTENSION pgrdf UPDATE,pgrdf.version() == extversion == 0.6.33):Zero
shmem_reset()calls in either proof.Release
Cargo.toml/lock,pgrdf.control, compose per-file mountsql/pgrdf--0.6.32--0.6.33.sqlupgrade bridge (comment-only — behaviour ships in the.so, no DDL)