Re: Deadlock detector fails to activate on a hot standby replica

Hi Vitaly,

Thanks for updating the patch!

On Wed, Aug 5, 2026 at 4:10 PM Vitaly Davydov <v(dot)davydov(at)postgrespro(dot)ru> wrote:
>
> Dear Xuneng Zhou, All
>
> I've updated the patch for the master branch taking into account Xuneng Zhou's
> review notes. Please, take a look.
>
> Key notes are:
>
> 1. Renamed BufferIsReadyForCleanup to PinCountWaiterCheckReadyForCleanup and
> removed some asserts just after LockBufHdr to avoid stalling of other processes
> accessing this buffer if assert is activated. I'm not sure what are the adopted
> rules for asserts - should we avoid indefinite locks or other issues due to
> assert activation in general?
>
> 2. Removed elog(ERROR) from RegisterPinCountWaiter. I kept the asserts
> unchanged but I'm not sure about it.
>
> 3. Fixed the issue with fast path in ResolveRecoveryConflictWithBufferPin. I
> reused got_standby_delay_timeout variable in fast path when current time is
> greater than ltime. There is no need to check this condition every time in
> the for() loop. Checking the condition once and set got_standby_delay_timeout
> is enough.

I haven't looked into the details yet. This version seems not resolve
the awkwardness of dependency you pointed out earlier. Just wondering
the reason for not aiming higher. It seems not trivial but still
feasible. Do you think we better try to land the bug fix first and do
further refactoring based on that?

"I agree with your comment. Furthermore, I do not like the approach with
locking and unlocking in different functions. But, I see this approach
is used in some other places in the code. I should think how to improve it.

I have some doubts about the code where LockBufferForCleanup (bufmgr.c)
calls ResolveRecoveryConflictWithBufferPin (standby.c), but the latter
function calls BufferIsReadyForCleanup (bufmgr.c). There is an idea to
refactor it in the future, because these functions are closely coupled.
May be unite them or move ResolveRecoveryConflictWithBufferPin into
bufmgr.c..."

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.