Hi, On Fri, Jun 5, 2026 at 7:58 PM Fujii Masao <[email protected]> wrote: > > On Wed, May 27, 2026 at 12:01 AM Fujii Masao <[email protected]> wrote: > > + if (got_standby_delay_timeout) > > + SendRecoveryConflictWithBufferPin(RECOVERY_CONFLICT_BUFFERPIN); > > + else if (got_standby_deadlock_timeout) > > + { > > > > Shouldn't we break out of the loop when either got_standby_delay_timeout or > > got_standby_deadlock_timeout becomes true? Otherwise, the loop continues > > with > > those flags still set, which could cause > > SendRecoveryConflictWithBufferPin() to > > be called unnecessarily in the subsequent cycles. > > > > > > + if (BufferGetRefCount(buffer) <= 1) > > > > Should this be "BufferGetRefCount(buffer) == 1" instead? I don't think > > BufferGetRefCount(buffer) should ever return 0 here. If that's correct, > > would it make sense to explicitly detect that case, for example: > > > > ----------------- > > uint32 refcount = BufferGetRefCount(buffer); > > > > Assert(refcount > 0); > > > > if (refcount == 0) > > elog(ERROR, "buffer refcount dropped to zero while waiting for > > cleanup lock"); > > > > if (refcount == 1) > > break; > > ----------------- > > I've updated the patch based on these comments. > Attached is the latest version. > > I removed the TAP test from this patch for now. I'll consider adding > a test for this separately later. > > BTW, I'm just wondering whether ResolveRecoveryConflictWithLock() > might have the same issue. I need to investigate that further.
While working on something closely related, I revisited this thread. ResolveRecoveryConflictWithLock seems to also suffer from the same issue per investigation. I'll share some findings later. -- Regards, Xuneng Zhou HighGo Software Co., Ltd.
