From a666381e25a998bee5e8f458756e590e335fa6c5 Mon Sep 17 00:00:00 2001 From: Vadim Ponomarev Date: Sat, 15 Aug 2026 12:08:53 +0300 Subject: [PATCH v1 2/4] Compute the synced positions before taking the sync-rep queue lock SyncRepReleaseWaiters() takes SyncRepLock and only then walks the walsender slots to work out the synced write, flush and apply positions. That walk takes a spinlock per slot, allocates, and for a quorum set sorts the result, and every cycle of it is spent in the section every committer lines up on. The comment there conceded the work does not need the lock and kept it inside anyway, to guarantee the positions are newer than any previous execution of the routine used. That guarantee is not needed. The three sites that consume the positions each move lsn[] forward only when the new reading is ahead of the stored one, so positions gone stale while the lock was being taken release nobody and change nothing; a concurrent walsender that got further has already stored its own. Compute them before taking the lock, and leave without taking it at all when this walsender turns out not to be a sync standby. --- src/backend/replication/syncrep.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index ccd51022a5f..6406168721b 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -514,20 +514,15 @@ SyncRepReleaseWaiters(void) } /* - * We're a potential sync standby. Release waiters if there are enough - * sync standbys and we are considered as sync. - */ - wakelist = SyncRepWakeList(); - - LWLockAcquire(SyncRepLock, LW_EXCLUSIVE); - - /* - * Check whether we are a sync standby or not, and calculate the synced - * positions among all sync standbys. (Note: although this step does not - * of itself require holding SyncRepLock, it seems like a good idea to do - * it after acquiring the lock. This ensures that the WAL pointers we use - * to release waiters are newer than any previous execution of this - * routine used.) + * We're a potential sync standby. Check whether we are a sync standby + * and calculate the synced positions among all sync standbys before + * taking the lock: the walk over the walsender slots takes their + * spinlocks, allocates, and possibly sorts, and doing all of it under + * SyncRepLock delays every committer. + * + * Positions gone stale by the time the lock is held cost nothing. The + * guards further down only ever move lsn[] forward, so a reading older + * than a concurrent walsender's simply releases nobody. */ got_recptr = SyncRepGetSyncRecPtr(&writePtr, &flushPtr, &applyPtr, &am_sync); @@ -555,11 +550,14 @@ SyncRepReleaseWaiters(void) */ if (!got_recptr || !am_sync) { - LWLockRelease(SyncRepLock); announce_next_takeover = !am_sync; return; } + wakelist = SyncRepWakeList(); + + LWLockAcquire(SyncRepLock, LW_EXCLUSIVE); + /* * Set the lsn first so that when we wake backends they will release up to * this location. -- 2.34.1