On Sat, Oct 10, 2026 at 5:24 PM Xuneng Zhou <[email protected]> wrote:
>
> Hi Vitaly,
>
> On Fri, Sep 4, 2026 at 10:16 PM Vitaly Davydov <[email protected]> wrote:
> >
> > Hi Xuneng Zhou,
> >
> > > * The caller must already be registered as the shared buffer's
> > > + * BM_PIN_COUNT_WAITER.
> > > This line of comment for PinCountWaiterCheckReadyForCleanup in v8
> > > seems not accurate to me. RegisterPinCountWaiter() explicitly permits
> > > the shared bit BM_PIN_COUNT_WAITER to be absent.
> >
> > Agreed, thank you. Your edits to the comment for
> > PinCountWaiterCheckReadyForCleanup() look great.
> >
> > > The invariante seems to be: Assert(PinCountWaitBuf == bufHdr);
> > > This process owns the logical cleanup wait for this buffer.
> >
> > Yes, this invariant is checked in PinCountWaiterCheckReadyForCleanup().
> >
> > > Updated this, and extended the commit message like we discussed
> > > earlier though it might not get used at the end.
> >
> > Thank you very much for such an excellent commit message. Prior to it
> > we discussed to unite the explanation in one single paragraph. It seems,
> > I did poorly and I missed the description of changes. Your commit looks
> > very descriptive and complete.
>
> Thanks for proof-reading it!
>
> > One note about the commit message is the combining of words with a hyphen.
> > I see, that hot-standby appears in commit messages, but the hyphen in
> > buffer-pin, recovery-conflict or startup-progress seems excessive.
> >
> > I'm not sure, but the commit title ("Fix premature wakeups...") might
> > suggest
> > that the wakeups are being prevented entirely, rather than fixing how they
> > are handled.
>
> Ok, that's right. Agreed that the header line is somewhat misleading
> and needs to be softened. We did not *prevent* the general unrelated
> premature wakeups in ProcWaitForSignal. And those hyphens are
> unnecessary, they are removed. I also made some minor adjustments to
> the patch. Add timeouts_armed flag to tell the already expired timeout
> to avoid the extra round of cancellation request without
> re-introducing the original problem. If it is set, break the for loop,
> let the resolver send another cancellation request if the conflict is
> still there.
>
> There's another counterpart case for ResolveRecoveryConflictWithLock
> as the attached reproducer 913 shows. The startup process waits for a
> lock held by a standby session, which in turn waits for another lock
> held by the startup process. An unrelated wakeup before the set
> deadlock timeout causes the same kernel timeout rearming issue,
> preventing the startup process from detecting and breaking the cycle.
>
> > > The timeout para in that message is basically a summary of the first
> > email
> > > in this thread. I am not that familiar with it and too tired to
> > proof-read it.
> > > Can you do me a favor?
> >
> > The paragraph looks technically sound and explains the core issue in detail.
> > Previously, a hang could occur after just a single premature wake-up.
> > This is
> > now stated clearly. While interference from multiple active timeouts can
> > lead
> > to more complex behavior, the fundamental problem remains unchanged.
>
>
>
> > > Just wondering whether we need a test for it.
> >
> > I have a test for issue reproduction that is attached to the first email and
> > the patch in [1] where test/recovery/t/031_recovery_conflict.pl was
> > updated to reproduce the issue. But it uses log_startup_progress_interval
> > to activate a premature wakeup. There is an opinion in [2] that
> > log_startup_progress_interval behaviour may be changed, that invalidates
> > the test. I have no ideas about other simple alternative ways to trigger
> > premature wakeups. May be, injection points may help.
> >
> > There is a concern in [2] that changes to the behavior of
> > log_startup_progress_interval will invalidate the test. I cannot think of
> > other simple ways to trigger premature wakeups. Perhaps injection points
> > could help here.
> >
> > [1]
> > https://www.postgresql.org/message-id/CAHGQGwFED52rRVb_a%3DbVSdbOA%3DoARVOUyw381GC_6uKoNWJAtQ%40mail.gmail.com
> > [2]
> > https://www.postgresql.org/message-id/CAHGQGwHh5j%3DsG3AZh%3DHyuXtPM6cmymp6MJkRAMJcJUyN2qpHrQ%40mail.gmail.com
>
> pg_log_backend_memory_contexts seems able to trigger the premature
> wakeups for the startup. V10 adds two tests(lock/buffer pin) based on
> that. As usual, the hard part is to make it deterministic end-to-end.
>
> To make the stall occur, we need this sequence:
>
> 1) Startup sets a deadlock deadline and a kernel alarm for it.
> 2) An unrelated wakeup makes broken code move the deadline later,
> leaving the old alarm pending.
> 3) If startup handles that alarm before the new deadline, the handler
> wakes startup and schedules another alarm without firing the deadlock
> timeout.
> 4) Broken code moves the deadline later again, leaving that alarm pending.
> 5) Steps 3–4 can repeat indefinitely.
>
> The tricky part is in step 3. If there's some delay for the startup
> handling the alarm due to scheduling or other factors, the woken
> process would possibly find that the logical deadline has expired,
> subsequently, the handler would fire the deadlock timeout to send the
> cancellation request to break the cycle. There's no facility in the
> tree to simulate the time to let it 'pause' until the startup handles
> the alarm. Therefore, the added test exercises the mechanism rather
> than the behavior -- the timeout would not be rearmed after unrelated
> wakeups.
Sorry for the missing reproducer.
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
# Reproducer: a hot standby deadlock between the startup process, waiting
# for a lock held by a session, and that session, waiting for a lock that the
# startup process holds, is never detected after an unrelated wakeup of the
# startup process. With max_standby_streaming_delay = -1, recovery is stuck.
#
# A standby session holds a lock on table1 and waits for a lock on table2
# that the startup process holds for a prepared transaction. Its own
# deadlock check runs before replay waits for table1, and finds nothing.
# Replay of LOCK TABLE table1 then makes the startup process wait for the
# session. Now only the startup process can find the deadlock: after
# deadlock_timeout, it asks the session to check again.
#
# Any wakeup that does not end the startup process's wait makes it cancel its
# timers and start a new deadlock timeout. The kernel alarm for the old one
# stays pending and wakes it again before the new one expires, and so on, so
# the request is never sent. Here a memory-context request provides the
# first wakeup.
#
# The assertions below describe the deadlock, so they pass on affected
# servers.
use strict;
use warnings FATAL => 'all';
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
my $primary = PostgreSQL::Test::Cluster->new('primary');
$primary->init(allows_streaming => 1);
$primary->append_conf('postgresql.conf', 'max_prepared_transactions = 10');
$primary->start;
$primary->backup('backup');
# Disable the startup progress timer, so that the memory-context request is
# the only wakeup.
my $standby = PostgreSQL::Test::Cluster->new('standby');
$standby->init_from_backup($primary, 'backup', has_streaming => 1);
$standby->append_conf(
'postgresql.conf', q[
max_standby_streaming_delay = -1
deadlock_timeout = 3s
log_recovery_conflict_waits = on
log_startup_progress_interval = 0
]);
$standby->start;
# Make the startup process hold an AccessExclusiveLock on table2.
$primary->safe_psql(
'postgres', q[
CREATE TABLE table1(a int);
CREATE TABLE table2(a int);
BEGIN;
LOCK TABLE table2;
PREPARE TRANSACTION 'lock';
]);
$primary->wait_for_replay_catchup($standby);
# The session locks table1, then waits for the lock on table2. Make its own
# deadlock check run quickly, and log that it ran, so that it finishes before
# the startup process waits for table1.
my $session = $standby->background_psql('postgres', on_error_stop => 0);
my $session_pid = $session->query_safe('SELECT pg_backend_pid()');
my $log_offset = -s $standby->logfile;
$session->query_until(
qr/^1$/m, q[
BEGIN;
SET LOCAL deadlock_timeout = '10ms';
SET LOCAL log_lock_waits = on;
LOCK TABLE table1 IN ACCESS SHARE MODE;
SELECT 1;
SELECT * FROM table2;
]);
$standby->wait_for_log(
qr/process $session_pid still waiting for AccessShareLock/, $log_offset);
# Replaying the lock on table1 makes the startup process wait for the
# session.
$primary->safe_psql('postgres', 'BEGIN; LOCK TABLE table1; COMMIT;');
$standby->wait_for_event('startup', 'relation');
ok(1, 'startup process waits for the session');
# Wake the startup process before its deadlock timeout expires.
my $startup_pid = $standby->safe_psql('postgres',
"SELECT pid FROM pg_stat_activity WHERE backend_type = 'startup'");
sleep(1);
$standby->safe_psql('postgres',
"SELECT pg_log_backend_memory_contexts($startup_pid)");
$standby->wait_for_log(qr/logging memory contexts of PID $startup_pid\b/,
$log_offset);
# deadlock_timeout passes several times, but the deadlock is not resolved.
sleep(10);
is( $standby->safe_psql(
'postgres',
"SELECT wait_event FROM pg_stat_activity WHERE backend_type = 'startup'"
),
'relation',
'startup process is still waiting for the session');
is( $standby->safe_psql(
'postgres',
"SELECT wait_event_type || ':' || wait_event FROM pg_stat_activity WHERE pid = $session_pid"
),
'Lock:relation',
'session is still waiting for the startup process');
unlike(
slurp_file($standby->logfile, $log_offset),
qr/ERROR: deadlock detected/,
'no deadlock was detected');
# Break the cycle by canceling the session.
$standby->safe_psql('postgres', "SELECT pg_cancel_backend($session_pid)");
$primary->wait_for_replay_catchup($standby);
ok(1, 'replay resumes once the session is canceled');
$session->quit;
$primary->safe_psql('postgres', q[ROLLBACK PREPARED 'lock']);
done_testing();
