From b435d68a0321cb384d105f9f5f42a79f48cdefe9 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Mon, 7 Sep 2026 13:27:11 +0300 Subject: [PATCH v2 1/3] Prevent WAIT FOR LSN from deadlocking recovery A backend waiting for a standby LSN can retain locks acquired by earlier statements. The startup process may need one of those locks, directly or through another backend, before it can advance replay. The backend's latch-based LSN wait is invisible to the deadlock detector -- it sleeps on its latch rather than in ProcSleep(), so the startup deadlock probe finds GetAwaitedLock() == NULL and returns without acting -- and the resulting cycle can persist indefinitely with unlimited standby conflict delays. Write and flush waits are restricted as well. Their positions are floored by the replay position, so during archive recovery the startup process is their only source of progress. Streaming does advance them independently, but only while WAL keeps arriving: a blocked startup process can neither restart a walreceiver that has stopped nor create the restartpoints that recycle WAL, so the standby fills pg_wal and then loses reception anyway. The independence streaming provides is therefore bounded by free space in pg_wal, not durable for the life of the wait. Reject an unsatisfied standby_replay, standby_write, or standby_flush wait while recovery is active when the backend already holds a granted lock. This conservative restriction covers direct relation-lock cycles and indirect cycles involving advisory locks. Waits whose target has already been reached return without sleeping and stay allowed, as do primary_flush waits and waits issued after recovery has ended. Report the offending lock so the user can find it, using the same description the deadlock report uses. Add 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 | 36 +++++++++++++++++++++ src/backend/commands/wait.c | 42 +++++++++++++++++++++++++ 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, 147 insertions(+) diff --git a/doc/src/sgml/ref/wait_for.sgml b/doc/src/sgml/ref/wait_for.sgml index 04ca9400426..49ca0b6c02d 100644 --- a/doc/src/sgml/ref/wait_for.sgml +++ b/doc/src/sgml/ref/wait_for.sgml @@ -225,6 +225,42 @@ 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 during archive recovery recovery is their + only source of progress. Under streaming replication they advance only + while WAL keeps arriving, and a startup process that is blocked can neither + restart a walreceiver that has stopped nor create the restartpoints that + recycle WAL, so pg_wal eventually fills up and + reception stops as well. + + 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..c08a4c9c153 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,46 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, "Use standby_flush mode on a standby server."))); } + /* + * A standby wait that is not already satisfied must not retain heavyweight + * locks. Such a lock can make the startup process wait for us, directly + * or through another backend, while we wait for it to advance recovery. + * That cycle is invisible to the deadlock detector, because we sleep on + * our latch rather than in ProcSleep(), so nothing breaks it. + * + * Write and flush waits are restricted as well. Their positions are + * floored by the replay position, so during archive recovery the startup + * process is their only source of progress. Streaming does advance them + * independently, but only while WAL keeps arriving: a blocked startup + * process cannot restart a walreceiver that has stopped, and it also stops + * producing the restartpoints that recycle WAL, so the standby fills + * pg_wal and then loses reception anyway. Rather than guess how long that + * lasts, treat all standby modes alike. + */ + 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 cf5c98c8176..11fa6717430 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -673,6 +673,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.55.0