Re: Delay of standby shutdown - Mailing list pgsql-hackers

Hello Fujii,

On Thu, Sep 17, 2020 at 6:49 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

As far as I understand after debugging, the problem is as follows:

1. After the primary is stopped, and the standby startup process is
waiting inside:

(void) WaitLatch(&XLogCtl->recoveryWakeupLatch,
WL_LATCH_SET | WL_TIMEOUT |
WL_EXIT_ON_PM_DEATH,
wait_time,
WAIT_EVENT_RECOVERY_RETRIEVE_RETRY_INTERVAL);

it receives SIGTERM on account of stopping the standby and it leads to
the WaitLatch call returning with WL_LATCH_SET.

2. WaitForWALToBecomeAvailable() then will return true after calling
XLogFileReadAnyTLI() and eventually, XLogReadRecord() will return NULL
since there is no new WAL to read, which means ReadRecord() will loop
back and perform another XLogReadRecord().

3. The additional XLogReadRecord() will lead to a 5s wait inside
WaitForWALToBecomeAvailable() - a different WaitLatch() call this time:

/*
 * Wait for more WAL to arrive. Time out after 5 seconds
 * to react to a trigger file promptly and to check if the
 * WAL receiver is still active.
 */
(void) WaitLatch(&XLogCtl->recoveryWakeupLatch,
WL_LATCH_SET | WL_TIMEOUT |
WL_EXIT_ON_PM_DEATH,
5000L, WAIT_EVENT_RECOVERY_WAL_STREAM);

4. And then eventually, the code will handle interrupts here inside
WaitForWALToBecomeAvailable() after the 5s wait:

/*
 * This possibly-long loop needs to handle interrupts of startup
 * process.
 */
HandleStartupProcInterrupts();

> To avoid this issue, I think that ReadRecord() should call
> HandleStartupProcInterrupts() whenever looping back to retry.

Alternatively, perhaps we can place it inside
WaitForWALToBecomeAvailable() (which already has a similar call),
since it is more semantically aligned to checking for interrupts, rather
than ReadRecord()? Like this:

diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 52a67b117015..b05cf6c7c219 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -12249,6 +12249,10 @@ WaitForWALToBecomeAvailable(XLogRecPtr
RecPtr, bool randAccess,

WAIT_EVENT_RECOVERY_RETRIEVE_RETRY_INTERVAL);
                         ResetLatch(&XLogCtl->recoveryWakeupLatch);
                         now = GetCurrentTimestamp();
+                        /*
+                         * Check for interrupts
+                         */
+                        HandleStartupProcInterrupts();
                     }
                     last_fail_time = now;
                     currentSource = XLOG_FROM_ARCHIVE;

It also has the advantage of being in a slightly less "hot" code path
as compared to it being where you suggested (the location you suggested
is executed infinitely when a standby is not connected to a primary and
there is no more WAL to read)


Regards,

Soumyadeep and Georgios