MIME-Version: 1.0
References: <44c24dcf-5710-410f-b1b6-d10b315f3d51@postgrespro.ru>
 <b178ea8d-9ed9-48b3-b4f7-5cfc3ff6ee44@postgrespro.ru>
In-Reply-To: <b178ea8d-9ed9-48b3-b4f7-5cfc3ff6ee44@postgrespro.ru>
From: Fujii Masao <masao.fujii@gmail.com>
Date: Thu, 21 May 2026 01:26:53 +0900
Message-ID: 
 <CAHGQGwExVv1-mtPEX-JFGBXNY2r8DfP+sQVPD8GpDTefW0KdsQ@mail.gmail.com>
Subject: Re: Deadlock detector fails to activate on a hot standby replica
To: Vitaly Davydov <v.davydov@postgrespro.ru>
Cc: pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAHGQGwExVv1-mtPEX-JFGBXNY2r8DfP%2BsQVPD8GpDTefW0KdsQ%40mail.gmail.com>
Precedence: bulk

On Fri, Jan 23, 2026 at 8:52=E2=80=AFPM Vitaly Davydov <v.davydov@postgresp=
ro.ru> wrote:
>
> Dear Hackers,
>
> I would like to propose a patch that fixes the problem, which has the roo=
ts in
> the possibility of spontaneous SIGALRM signals when waiting for some time=
outs.
> The idea of the patch - ignore spontaneous SIGALRM signals and continue w=
aiting
> for expected timeouts or buffer unpinning by the conflicting backend. Thi=
s
> patch is not a final version. I plan to add a tap-test for this case.

Thanks for the patch!


 #include "storage/sinvaladt.h"
 #include "storage/standby.h"
+#include "storage/buf_internals.h"

This include should be placed in alphabetical order.


+ * We assume that only UnpinBuffer() and the timeout requests established
+ * above can wake us up here. WakeupRecovery() called by walreceiver or
+ * SIGHUP signal handler, etc cannot do that because it uses the different
+ * latch from that ProcWaitForSignal() waits on.

As your investigation showed, that assumption does not seem to hold. If so,
I think something like the following would be more accurate:

    ---------------------------------
    ProcWaitForSignal() can also wake up for unrelated reasons, so recheck
    whether we're still the waiter after each wakeup. If we are and no time=
out
    fired, continue waiting without resetting the active timeouts.
    ---------------------------------


+ uint32 buf_refcount =3D BUF_STATE_GET_REFCOUNT(buf_state);
<snip>
+ if (buf_refcount > 1)
+ continue;

Wouldn't it be better to check explicitly whether we're still the waiter,
instead of using BUF_STATE_GET_REFCOUNT()?

    (buf_state & BM_PIN_COUNT_WAITER) !=3D 0 &&
    bufHdr->wait_backend_pgprocno =3D=3D MyProcNumber


The current control flow in the loop feels a bit hard to follow.
Would something like the following be simpler?

    for (;;)
    {
        ....
        ProcWaitForSignal(...);

        if (!StillWaitingForBufferPin(...))
            break;

        if (got_standby_delay_timeout)
        {
            SendRecoveryConflictWithBufferPin(...);
            break;
        }
        else if (got_standby_deadlock_timeout)
        {
            SendRecoveryConflictWithBufferPin(...);
            break;
        }
    }

Regards,

--=20
Fujii Masao