From 5e0266d7d0d65a098cc9c200a9cc3bdda9cf0ebc Mon Sep 17 00:00:00 2001 From: Xuneng Zhou Date: Wed, 9 Sep 2026 10:56:40 +0800 Subject: [PATCH v3 1/4] Prevent WAIT FOR LSN from deadlocking recovery on held locks A backend waiting for WAL replay can retain locks acquired by earlier statements. If the startup process needs one of those locks, directly or through another backend, before reaching the target LSN, a deadlock can arise: startup waits for the backend to release the lock, while the backend waits for startup to advance replay. The lock manager records the backend's held locks, but WAIT FOR LSN does not register its dependency on replay as a lock wait. The deadlock detector therefore cannot see the complete cycle. With unlimited standby conflict delays and no other timeout or cancellation, this deadlock can persist indefinitely. Write and flush waits are restricted as well. Their positions are floored by the replay position, so without an active walreceiver the startup process can be their only source of progress. If a held lock blocks replay, these waits can form the same cycle: the backend waits for replay to advance, while replay waits for the backend to release the lock. Streaming does advance them independently, but only while WAL keeps arriving. If reception stops before the target is reached, a blocked startup process cannot restart the walreceiver. It also cannot replay newer checkpoint records needed to advance restartpoints and recycle WAL, so continued reception can exhaust available space in pg_wal before the target is reached. An active receiver at the start of the wait therefore does not guarantee that the wait can finish while replay remains blocked. Reject an unsatisfied standby_replay, standby_write, or standby_flush wait while recovery is active when the backend already holds a granted heavyweight lock. This conservative restriction covers direct relation-lock cycles and indirect cycles involving advisory locks. It also rejects some write and flush waits that an active receiver could satisfy. Requests whose target is observed as already reached are exempt from this check, as are primary_flush requests and requests issued after recovery has ended. Existing snapshot and recovery-state checks still apply. Report one of the held locks so the user can find it, using the same description the deadlock report uses. Add replay-mode tests for relation and advisory locks and for the already-reached case, and document the restriction along with the recommended usage pattern. Author: Xuneng Zhou Discussion: https://postgr.es/m/CABPTF7U0gW5%2B-4oL7-qdML-yerZxUb7ku4QXp7JxCYo0qyJ_Tw%40mail.gmail.com Reviewed-by: Alexander Korotkov Backpatch-through: 19 --- doc/src/sgml/ref/wait_for.sgml | 40 ++++++++++++++++++ src/backend/commands/wait.c | 55 +++++++++++++++++++++++++ src/backend/storage/lmgr/lock.c | 32 ++++++++++++++ src/include/storage/lock.h | 1 + src/test/recovery/t/049_wait_for_lsn.pl | 36 ++++++++++++++++ 5 files changed, 164 insertions(+) diff --git a/doc/src/sgml/ref/wait_for.sgml b/doc/src/sgml/ref/wait_for.sgml index 04ca9400426..0042825e702 100644 --- a/doc/src/sgml/ref/wait_for.sgml +++ b/doc/src/sgml/ref/wait_for.sgml @@ -225,6 +225,46 @@ WAIT FOR LSN 'lsn' at isolation levels higher than READ COMMITTED. + + While recovery is in progress, a wait in standby_replay + (the default), standby_write, or + standby_flush mode is rejected when the session already + holds a lock and the target lsn has not been reached + yet. Such a lock can make the startup process wait for this session, either + directly or through another session, while this session waits for the + startup process to advance recovery. That cycle involves no lock wait on + this side, so deadlock detection does not see it and nothing breaks it. A + wait whose target has already been reached returns immediately and is + therefore always allowed. + + + + Issue WAIT FOR outside a transaction block, or as the + first statement of one, before running anything that takes locks. That is + also the natural order for the read-your-writes pattern shown in the + examples below: wait for the target lsn first, then + run the queries that have to see it. Note that a lock taken by an earlier + statement is still held at READ COMMITTED, even though + its snapshot is gone, so a wait placed after such a statement is rejected + even when the isolation level permits it. + + + + The restriction covers standby_write and + standby_flush as well, even though streaming replication + can advance those positions without the startup process. Both positions are + at least the replay position, so without an active walreceiver replay can + be their only source of progress. If a held lock blocks replay, the session + waits for replay to advance while replay waits for the session to release + the lock. Under streaming replication the positions advance independently + only while WAL keeps arriving. If reception stops before the target is + reached, a blocked startup process cannot restart the walreceiver. It also + cannot replay newer checkpoint records needed to advance restartpoints and + recycle WAL, so pg_wal can fill up and reception can + stop before the target is reached. The restriction therefore also applies + when streaming is active at the start of the wait. + + WAIT FOR waits until the specified lsn is reached according to the specified diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c index 9ba4c75021e..90cc4c73c1d 100644 --- a/src/backend/commands/wait.c +++ b/src/backend/commands/wait.c @@ -23,6 +23,8 @@ #include "commands/wait.h" #include "executor/executor.h" #include "parser/parse_node.h" +#include "storage/lmgr.h" +#include "storage/lock.h" #include "storage/proc.h" #include "utils/builtins.h" #include "utils/guc.h" @@ -194,6 +196,59 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, "Use standby_flush mode on a standby server."))); } + /* + * Conservatively reject an unsatisfied standby LSN wait while this + * backend holds a granted heavyweight lock. Recovery may need one of + * those locks, directly or through another backend, before replay can + * advance far enough to satisfy our wait. This can create a cycle: + * we wait for recovery, while recovery waits for us to release the lock. + * + * WAIT FOR LSN does not register its dependency on WAL progress as a + * lock wait, so the deadlock detector cannot see the complete cycle. + * With unlimited recovery-conflict delays and no other timeout or + * cancellation, the cycle can persist indefinitely. + * + * Write and flush waits can also depend on startup. Without an active + * receiver, their replay floor can be their only source of progress, + * so holding a lock needed by replay can create the same cycle. + * + * Streaming can initially provide independent progress, but reception + * can stop before the target is reached. Restarting reception requires + * startup, and stalled replay prevents further advancement of + * restartpoints used to recycle old WAL, so continued reception can + * exhaust available space. An active receiver at the start of the wait + * therefore does not guarantee that the wait can finish while replay + * remains blocked. + * + * Apply the restriction to all standby modes, including some write and + * flush waits that an active receiver could satisfy while locks remain + * held. Requests whose target is observed as already reached are exempt + * from this restriction. + */ + if ((lsnType == WAIT_LSN_TYPE_STANDBY_REPLAY || + lsnType == WAIT_LSN_TYPE_STANDBY_WRITE || + lsnType == WAIT_LSN_TYPE_STANDBY_FLUSH) && + RecoveryInProgress() && + lsn > GetCurrentLSNForWaitType(lsnType)) + { + LOCKTAG locktag; + + if (GetAnyGrantedHeavyweightLock(&locktag)) + { + StringInfoData locktagbuf; + + initStringInfo(&locktagbuf); + DescribeLockTag(&locktagbuf, &locktag); + + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot wait for a standby LSN while holding locks"), + errdetail("This session holds a lock on %s, which could make recovery wait for this session while this session waits for recovery.", + locktagbuf.data), + errhint("Release the locks, or execute WAIT FOR before acquiring them."))); + } + } + /* Now wait for the LSN */ waitLSNResult = WaitForLSN(lsnType, lsn, timeout); diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index b8a24544fd7..00978168bbf 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -669,6 +669,38 @@ LockHeldByMe(const LOCKTAG *locktag, return false; } +/* + * GetAnyGrantedHeavyweightLock -- find some heavyweight lock this backend owns + * + * Returns true and stores the tag of an arbitrary granted lock in *locktag if + * the local lock table represents any lock we own, false otherwise. Which + * lock is reported is unspecified when we hold several; callers use it only to + * give the user a starting point. + * + * A LOCALLOCK entry can remain after an unsuccessful lock acquisition, so only + * entries with a positive local hold count represent locks we own. + */ +bool +GetAnyGrantedHeavyweightLock(LOCKTAG *locktag) +{ + HASH_SEQ_STATUS status; + LOCALLOCK *locallock; + + hash_seq_init(&status, LockMethodLocalHash); + + while ((locallock = (LOCALLOCK *) hash_seq_search(&status)) != NULL) + { + if (locallock->nLocks > 0) + { + *locktag = locallock->tag.lock; + hash_seq_term(&status); + return true; + } + } + + return false; +} + #ifdef USE_ASSERT_CHECKING /* * GetLockMethodLocalHash -- return the hash of local locks, for modules that diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index ee3cb1dc203..c63c9075466 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -401,6 +401,7 @@ extern void LockReleaseCurrentOwner(LOCALLOCK **locallocks, int nlocks); extern void LockReassignCurrentOwner(LOCALLOCK **locallocks, int nlocks); extern bool LockHeldByMe(const LOCKTAG *locktag, LOCKMODE lockmode, bool orstronger); +extern bool GetAnyGrantedHeavyweightLock(LOCKTAG *locktag); #ifdef USE_ASSERT_CHECKING extern HTAB *GetLockMethodLocalHash(void); #endif diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl index cb7d4d461de..1ba79951b80 100644 --- a/src/test/recovery/t/049_wait_for_lsn.pl +++ b/src/test/recovery/t/049_wait_for_lsn.pl @@ -431,6 +431,42 @@ $node_standby->psql( ok( $stderr =~ /conflicting or redundant options/, "get error for duplicate MODE parameter"); +# An unsatisfied standby_replay wait must be rejected when the backend holds +# a lock. The relation lock covers the direct two-process cycle, while the +# advisory lock is the return edge in the indirect three-process cycle. The +# short timeout keeps these tests bounded if the check regresses. +$node_standby->psql( + 'postgres', qq[ + BEGIN; + SELECT count(*) FROM wait_test; + WAIT FOR LSN '${lsn3}' WITH (timeout '100ms');], + stderr => \$stderr); +like( + $stderr, + qr/cannot wait for a standby LSN while holding locks/, + "reject replay wait while holding a relation lock"); + +$node_standby->psql( + 'postgres', qq[ + BEGIN; + SELECT pg_advisory_xact_lock(42); + WAIT FOR LSN '${lsn3}' WITH (timeout '100ms');], + stderr => \$stderr); +like( + $stderr, + qr/cannot wait for a standby LSN while holding locks/, + "reject replay wait while holding an advisory lock"); + +# A wait whose target has already been reached returns without sleeping, so it +# cannot join a cycle and stays allowed regardless of the locks held. +my $reached = $node_standby->safe_psql( + 'postgres', qq[ + BEGIN; + LOCK TABLE wait_test IN ACCESS SHARE MODE; + WAIT FOR LSN '${lsn1}';]); +is($reached, 'success', + "allow an already-reached wait while holding a relation lock"); + # 7a. Check the scenario of multiple standby_replay waiters. We make 5 # background psql sessions each waiting for a corresponding insertion. When # waiting is finished, stored procedures logs if there are visible as many -- 2.50.1 (Apple Git-155)