On Tue, Jun 23, 2026 at 10:44 AM Xuneng Zhou <[email protected]> wrote: > > On Tue, Jun 23, 2026 at 4:11 AM Andres Freund <[email protected]> wrote: > > > > Hi, > > > > On 2026-06-22 16:14:08 +0800, Xuneng Zhou wrote: > > > This looks like a real lost-wakeup race in LockBufferForCleanup(). > > > > I don't think any wakeup is lost. The problem is that we're waiting for a > > wakeup when it is not guaranteed that one will arrive. > > I think this is true. > > > > The relevant sequence in failed test 048 is: > > > > > > - Session B opens a cursor and fetches one heap tuple, leaving a heap > > > buffer pinned. > > > - Session A starts VACUUM (FREEZE). > > > - VACUUM reaches that page and waits in LockBufferForCleanup(). > > > - Session B later advances/closes the cursor, releasing the pin. > > > - VACUUM is expected to wake up and finish. > > > > > > In the failure, the tap test had already passed its three sql > > > assertions, but then it timed out waiting for VACUUM completion: > > > > > > ok 3 - Cursor query returned 7 from second fetch > > > poll_query_until timed out: > > > SELECT vacuum_count > 0 ... > > > last actual query output: f > > > Looks like your test exited with 29 just after 3. > > > > > > The diagnostic log shows the actual race: > > > > > > LOG: !!!LockBufferForCleanup| wakeup lost > > > CONTEXT: while scanning block 90 of relation > > > "public.vac_horizon_floor_table" > > > > > LOG: !!!LockBufferForCleanup]| before ProcWaitForSignal > > > CONTEXT: while scanning block 90 of relation > > > "public.vac_horizon_floor_table" > > > > I don't think this is meaningful output, as written upthread. > > I agree that the output could be misleading. The missing post-wait log > seems to be an indication of 'no one wakes us after falling asleep > unnecessary' in this specific test case since no later backend > pins/unpins that same table buffer. Sure this is just the symptom. > > > > We can fix this with the state returned by UnlockBufHdrExt() when > > > publishing BM_PIN_COUNT_WAITER. If the wait refcount is 1, do not > > > enter the wait path. Instead, fall through to the existing waiter-bit > > > cleanup and retry the loop to acquire the cleanup lock normally. The > > > reproducer test passed after applying the patch. > > > > Why are we retrying, when we already achieved everything we wanted? Seems we > > should just unset BM_PIN_COUNT_WAITER / wait_backend_pgprocno. I think the > > easiest way to do this would be to not unlock the buffer header lock, but > > just > > atomically-or-in BM_PIN_COUNT_WAITER and then recheck if we already are > > done, > > while still holding the header lock. > > Yeah, there's no need to retry. What you proposed looks cleaner and > better. I'll update the patch as suggested. Thanks!
Here is the updated patch. Please take a look. -- Regards, Xuneng Zhou HighGo Software Co., Ltd.
v2-0001-bufmgr-avoid-waiting-without-a-guaranteed-pin-cou.patch
Description: Binary data
