Skip to content

PG17: re-find the proclock after waiting instead of trusting a stale pointer - #84

Merged
akorotkov merged 1 commit into
patches17from
proclock-refind-after-wait-17
Aug 13, 2026
Merged

PG17: re-find the proclock after waiting instead of trusting a stale pointer#84
akorotkov merged 1 commit into
patches17from
proclock-refind-after-wait-17

Conversation

@akorotkov

Copy link
Copy Markdown
Contributor

PG17 counterpart of 2859252 on patches18 (ORI-247, PANIC: proclock table corrupted).

A plain cherry-pick is wrong here, for two reasons — both worth knowing
before reviewing:

1. The partition lock

On PG18 LockAcquireExtended() releases the partition lock before
WaitOnLock(), so that fix re-takes it around the lookup. On PG17
ProcSleep() still "must be held at entry, and will be held at exit" — it
re-acquires it LW_EXCLUSIVE immediately before returning. Taking it again
here would self-deadlock. So the lookup is a plain hash_search_with_hash_value
with no acquire.

2. The dontWait path

PG17 handles a failed conditional acquisition inside this same block, and that
path never slept: ProcSleep() with dontWait returns without ever releasing
the partition lock, so no one can have run RemoveFromWaitQueue() on us and the
captured pointers are still good. Worse, that path needs them — it deletes the
proclock and decrements lock->nRequested itself. The re-find is therefore done
only when we actually slept.

Also guarded the INCONSISTENT prints, which can now see a proclock that is
gone rather than merely one without our bit, and kept the local lock's cached
pointers in step on the granted path.

Validation

Same method as the PG18 fix, on a PG17 build with orioledb: a probe reporting a
proclock that was freed while we slept fires 157 times in 60 seconds of the
contended-upsert workload — 157 reads of freed shared memory that this change no
longer performs — with no panic and no assertion failure.

PG16 port follows separately; its block has no dontWait case, so it is
simpler.

PG17 counterpart of 2859252 on patches18 (ORI-247, "PANIC: proclock
table corrupted").  A plain cherry-pick is wrong here, for two reasons.

The partition lock.  On PG18 LockAcquireExtended() releases it before
WaitOnLock(), so that fix re-takes it around the lookup.  On PG17
ProcSleep() still takes the partition lock at entry and re-acquires it
EXCLUSIVE before returning, so it is held here; acquiring it again would
self-deadlock.  The lookup is therefore a plain hash_search.

The dontWait path.  PG17 handles a failed conditional acquisition in
this same block, and that path never slept -- ProcSleep() with dontWait
returns without releasing the partition lock, so nobody can have run
RemoveFromWaitQueue() on us and the captured pointers are still good.
Worse, that path *needs* them: it deletes the proclock and decrements
lock->nRequested itself.  So the re-find is done only when we actually
slept.

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 fix: with a probe reporting a proclock
that was freed while we slept, the contended-upsert workload against
orioledb hits it 157 times in 60 seconds on PG17 -- 157 reads of freed
shared memory that this change no longer performs -- and runs with no
panic and no assertion failure.
@akorotkov
akorotkov merged commit 1e13fa1 into patches17 Aug 13, 2026
8 checks passed
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.

1 participant