PG16: re-find the proclock after waiting instead of trusting a stale pointer - #85
Merged
Merged
Conversation
PG16 counterpart of 2859252 on patches18 (ORI-247, "PANIC: proclock table corrupted"). A plain cherry-pick does not apply: on PG18 LockAcquireExtended() releases the partition lock before WaitOnLock() and so the fix there re-takes it around the lookup, while on PG16 ProcSleep() holds it at entry and re-acquires it EXCLUSIVE before returning -- taking it again here would self-deadlock. The lookup is therefore a plain hash_search. Unlike PG17 there is no dontWait case in this block (a conditional acquisition returns earlier), so the re-find is unconditional. Also guard the INCONSISTENT prints, which can now see a proclock that is gone rather than merely one without our bit, and keep the local lock's cached pointers in step on the granted path. Verified the same way as the PG18 and PG17 fixes: with a probe reporting a proclock that was freed while we slept, the contended-upsert workload against orioledb hits it 161 times in 60 seconds on PG16 -- 161 reads of freed shared memory that this change no longer performs -- and runs with no panic and no assertion failure.
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
1 participant
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.
PG16 counterpart of 2859252 on
patches18(ORI-247,PANIC: proclock table corrupted). Companion to #84 (PG17).Why a cherry-pick does not apply. On PG18
LockAcquireExtended()releasesthe partition lock before
WaitOnLock(), so that fix re-takes it around thelookup. On PG16
ProcSleep()holds the partition lock at entry and re-acquiresit
LW_EXCLUSIVEbefore returning, so it is held here — acquiring it againwould self-deadlock. The lookup is a plain
hash_search_with_hash_value.Unlike PG17 there is no
dontWaitcase in this block (a conditionalacquisition returns earlier), so the re-find is unconditional — this port is
the simpler of the two.
Also guarded the
INCONSISTENTprints, which can now see a proclock that isgone rather than merely one without our bit, and kept the local lock's cached
pointers in step on the granted path.
Validation
Built PG16 with orioledb and ran the same contended-upsert workload used for
the PG18 and PG17 fixes, with a probe reporting a proclock that was freed while
we slept: 161 hits in 60 seconds — 161 reads of freed shared memory that
this change no longer performs — with no panic and no assertion failure.
For the record, the same measurement across the three branches: PG18 163,
PG17 157, PG16 161 hits per 60 s. The defect is equally live on all three.