public inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: JoongHyuk Shin <sjh910805@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Fujii Masao <masao.fujii@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] Prevent repeated deadlock-check signals in standby buffer pin waits
Date: Mon, 27 Apr 2026 20:07:17 +0900
Message-ID: <CACSdjfPpwYcMAs6PdT9LQtxSNoiMZjrSfTNe5bOEvVB105Hzww@mail.gmail.com> (raw)
In-Reply-To: <aecRYWJmfQnuVDBe@paquier.xyz>
References: <CACSdjfO3ox8i8N3KxGf3HThzX1vnRRkY2kq1m5qsvGxjcuM1jg@mail.gmail.com>
<CAHGQGwFqKOmLNiPhBYB0Bh1Akot=SbTpzMSHA-au4aqVa8PKog@mail.gmail.com>
<aecRYWJmfQnuVDBe@paquier.xyz>
Thanks for the review.
v2 attached, with the suggested initialization added for symmetry.
Agreed this is an improvement rather than a bug fix,
so I've updated the CF tag to Performance accordingly.
I also verified the fix locally on a primary-standby setup,
using the buffer-pin conflict scenario from src/test/recovery/t/
031_recovery_conflict.pl
(aborted INSERT + cursor on standby + VACUUM FREEZE on primary).
On master, strace showed 9 SIGUSR1 broadcasts to the conflicting backend
over a 10-second window (one per deadlock_timeout).
With the patch applied, only 1 broadcast over the same window.
Patch attached.
--
JoongHyuk Shin
On Tue, Apr 21, 2026 at 2:55 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Tue, Apr 21, 2026 at 02:42:38PM +0900, Fujii Masao wrote:
> > Since this change improves recovery-conflict behavior rather than fixing
> a bug,
> > it doesn't seem to need backpatching and we may need to wait until v20
> > development opens (probably July) before committing it.
>
> Yeah, this one is an improvement, not an actual bug, so let's wait for
> v20 if worth doing (I did not check it).
> --
> Michael
>
Attachments:
[application/octet-stream] v2-0001-Prevent-repeated-deadlock-check-signals-in-standb.patch (2.8K, 3-v2-0001-Prevent-repeated-deadlock-check-signals-in-standb.patch)
download | inline diff:
From e70a2fc74a63d4c1e3d1277e27a37b1e26710fff Mon Sep 17 00:00:00 2001
From: JoongHyuk Shin <sjh910805@gmail.com>
Date: Fri, 17 Apr 2026 16:58:31 +0900
Subject: [PATCH v2] Prevent repeated deadlock-check signals in standby buffer
pin waits
After sending RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK, the startup process
returned without waiting, so the caller's loop would fire another
deadlock_timeout and re-send the signal every interval. This added
unnecessary overhead in both the startup process and backends.
Fix by adding a ProcWaitForSignal() call after the deadlock-check
signal, mirroring the approach already used in the lock-conflict path
(commit 8900b5a9d59a). This ensures the signal is sent at most once
per deadlock_timeout period rather than repeatedly.
Additionally, reset got_standby_delay_timeout to false before enabling
STANDBY_TIMEOUT, for symmetry with the existing got_standby_deadlock_timeout
reset.
Remove the XXX comment that noted this problem.
---
src/backend/storage/ipc/standby.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 29af7733948..0dba1fb4289 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -818,6 +818,7 @@ ResolveRecoveryConflictWithBufferPin(void)
if (ltime != 0)
{
+ got_standby_delay_timeout = false;
timeouts[cnt].id = STANDBY_TIMEOUT;
timeouts[cnt].type = TMPARAM_AT;
timeouts[cnt].fin_time = ltime;
@@ -851,17 +852,19 @@ ResolveRecoveryConflictWithBufferPin(void)
/*
* Send out a request for hot-standby backends to check themselves for
* deadlocks.
- *
- * XXX The subsequent ResolveRecoveryConflictWithBufferPin() will wait
- * to be signaled by UnpinBuffer() again and send a request for
- * deadlocks check if deadlock_timeout happens. This causes the
- * request to continue to be sent every deadlock_timeout until the
- * buffer is unpinned or ltime is reached. This would increase the
- * workload in the startup process and backends. In practice it may
- * not be so harmful because the period that the buffer is kept pinned
- * is basically no so long. But we should fix this?
*/
SendRecoveryConflictWithBufferPin(RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK);
+
+ /*
+ * Wait here to be signaled by UnpinBuffer(), to prevent the
+ * subsequent ResolveRecoveryConflictWithBufferPin() call (from the
+ * caller's loop) from firing another deadlock_timeout and re-sending
+ * the deadlock-check signal. Without this, the signal would be sent
+ * every deadlock_timeout interval until the buffer is unpinned or
+ * ltime is reached.
+ */
+ got_standby_deadlock_timeout = false;
+ ProcWaitForSignal(WAIT_EVENT_BUFFER_CLEANUP);
}
/*
--
2.52.0
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sjh910805@gmail.com, michael@paquier.xyz, masao.fujii@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Prevent repeated deadlock-check signals in standby buffer pin waits
In-Reply-To: <CACSdjfPpwYcMAs6PdT9LQtxSNoiMZjrSfTNe5bOEvVB105Hzww@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox