Re: Reduce SyncRepLock contention on the commit path

17.08.2026 11:29, Vadim Ponomarev пишет:
> Hi hackers,
>
> On a primary with synchronous replication, every commit that wrote WAL
> goes through SyncRepWaitForLSN(), and every standby reply goes through
> SyncRepReleaseWaiters(). Both take SyncRepLock exclusively, although
> much of their work does not need to happen under the lock.
>
> The attached series moves that work out of the critical section. The
> four patches are independent; only 0003 changes observable behaviour.
>
> 0001 -- Wake released waiters after dropping the queue lock.
>
> SyncRepWakeQueue() sets each released backend's latch while holding
> SyncRepLock. Setting a latch may call kill(), so at high commit rates
> the walsender makes one syscall per released commit while committers
> queue on the same lock.
>
> The patch collects released procs in a list and wakes them after dropping
> the lock, as ProcArrayGroupClearXid() already does with ProcArrayLock.
> The unlink, write barrier, and state update remain under the lock: a
> waiter reads syncRepState without it and must not see itself as completed
> while still on the queue.
>
> 0002 -- Compute synced positions before taking the queue lock.
>
> SyncRepReleaseWaiters() takes SyncRepLock before scanning the walsender
> slots for synced write, flush, and apply positions. That scan takes a
> spinlock per slot, allocates memory, and sorts quorum results.
>
> The patch moves the scan before the lock. The consumers only move lsn[]
> forward, so a result that becomes stale while waiting for the lock simply
> does not advance it. The patch also avoids the lock when the walsender
> is not a synchronous standby.
>
> 0003 -- Release waiters once per drained batch of standby replies.
>
> ProcessStandbyReplyMessage() calls SyncRepReleaseWaiters() for every
> reply, even when several replies are already waiting in the socket.
> Except for the last one, each pass then uses positions that the next
> message immediately replaces.
>
> The patch marks a release as pending and runs it once after draining the
> socket. The main risk is losing a pending release on an early exit: no
> other process will wake the committers acknowledged by that reply.
>
> The goodbye, EOF, invalid and unexpected message paths all run the
> pending release before returning. A normal standby shutdown sends its
> final reply and goodbye together, so that path matters in practice.
> Errors while parsing a later message, including a torn message, go
> through WalSndErrorCleanup(), which releases waiters after dropping the
> locks.
>
> 0004 -- Skip the lock when the acknowledgement has already arrived.
>
> This patch mirrors lsn[] in an atomic watermark and checks it before
> SyncRepWaitForLSN() takes the lock. The watermark is updated under the
> lock immediately after lsn[]. Both only move forward, so a stale read
> may take the slow path unnecessarily but cannot skip a required wait.
>
> I am least sure that 0004 is worth the extra shared state; see below.
>
> Prior work
> ----------
>
> 0001 was proposed in Thomas Munro's "Latches vs lwlock contention"
> thread as part of a general SetLatches() facility:
>
> https://www.postgresql.org/message-id/CA%2BhUKGKmO7ze0Z6WXKdrLxmvYa%3DzVGGXOO30MMktufofVwEm1A%40mail.gmail.com
>
> The heavyweight-lock part of that work was committed in November 2024,
> but SetLatches() and its sync-rep use were not. As far as I can tell,
> the sync-rep patch did not get a separate review. I used a list local to
> syncrep.c to keep the change contained and avoid the open questions
> around buffers and allocation in the general facility.
>
> There is also precedent for 0004. Michael Paquier's 2e57790836c ("Fix
> race with synchronous_standby_names at startup", April 2025) reads
> WalSndCtl->sync_standbys_status without the lock. The same monotonicity
> argument applies here; the LSN needs the atomics API because it is 64
> bits wide.
>
> Measurements
> ------------
>
> I compared devel master with the same master plus all four patches on
> two hosts connected by a dedicated 100 GbE link (RTT 0.11 ms):
>
> primary 4-socket Xeon Platinum 8580, 240 threads, 2 TB RAM, NVMe
> standby 2-socket Xeon Gold 5320, 104 threads, 1 TB RAM, NVMe
>
> The test used pgbench scale 2000, fillfactor 70, 750 clients,
> -M prepared, the built-in TPC-B script, and 10-minute runs with
> synchronous_commit = on, fsync and full_page_writes on. postgres used
> two primary sockets and pgbench a third.
>
> Each point started from the same prepared cluster, and the standby was
> rebuilt from a fresh base backup. I ran three interleaved pairs,
> alternating base and patched:
>
> tps runs 122250/122591/123091 -> 134849/134447/132740
> mean tps 122644 -> 134012 +9.3%
> mean latency 6.085 ms -> 5.558 ms -8.7%
> failed transactions: none
>
> The spread was 0.7% for base and 1.6% for patched.
>
> pg_stat_activity samples taken every 5 seconds show the same effect.
> The queue lock and standby acknowledgement wait are both named SyncRep;
> wait_event_type separates them:
>
> LWLock/SyncRep samples 12222 -> 6912 -43%
> IPC/SyncRep samples 20699 -> 14694 -29%
>
> Per committed transaction, lock-wait samples fell from 0.0997 to 0.0516.
> The queue-lock wait was roughly halved while throughput rose by 9%. The
> standby acknowledgement wait also fell by 35% per transaction, as
> committers stopped queueing for SyncRepLock before waiting for the
> standby.
>
> To see which patches contributed, I also ran -DLWLOCK_STATS builds on a
> small single-host setup: 16 threads, scale 20, 128 clients, 30 seconds.
> Across two runs, base -> patched:
>
> walsender acquisitions/commit 0.900-1.106 -> 0.538-0.610
> backend acquisitions/commit 0.9983-0.9995 -> 0.977-0.984
> commits that blocked 9.34-10.20% -> 0.24-0.27%
>
> At 32 clients, blocked acquisitions fell from 37809 to 362.
>
> 0003 makes most of the difference: the walsender takes the lock roughly
> half as often. 0004 rarely takes its fast path, saving only 1.6-2.3% of
> backend acquisitions at 128 clients. With synchronous_commit = on, it
> can only catch a commit if an acknowledgement for a later transaction
> happens to cover its LSN.
>
> I also ran four interleaved, 60-second single-client pairs. Mean
> latency was 0.760 ms on base and 0.762 ms patched, so deferring the
> release to the end of a one-reply drain showed no measurable delay.
>
> Testing
> -------
>
> 0003 adds src/test/recovery/t/056_syncrep_release.pl for two cases where
> a deferred release could be lost:
>
> * A stopped walsender resumes after the standby has applied the commit
> and shut down, then drains the final reply and goodbye together.
>
> * An injection point after a drained reply stands in for a torn message.
> The walreceiver is stopped while replay advances from WAL already on
> disk, so the first reply after resume carries the apply position the
> committer needs.
>
> Notes
> -------
>
> The SyncRepReleaseWaiters() call on configuration reload is outside the
> reply drain and is unchanged by 0003.
>
> 0001 allocates a MaxBackends-sized wake list once per process in
> TopMemoryContext. I am open to changing that if there is a better fit.
>
> The patches are against 7e6e294e4e4.
>
>
> -------
>
> Review would be especially helpful on two points:
>
> * Did I miss any exit path from the reply drain that must run the
> deferred release?
>
> * Is 0004 worth its new atomic in WalSndCtlData for a 1.6-2.3% reduction
> in backend lock acquisitions in these tests?
>
> I plan to register 0001-0003 for the September CommitFest and drop 0004,
> unless there is a workload where its fast path is more useful.
>
> Regards,
> Vadim Ponomarev

Good day, Vadim.

We've measured your patches and confirm they improve performance of
synchronous replication:
- with couple of 56 vcore virtual machines and pgbench running on replica,
250 clients, scale 2000, "TPC-B like" improved from 67.3kTPS to 70.4kTPS.
Which is quite impressive.

0001 i've found independently, so I fully share the idea. It really works.

0003 impressed me a lot. Great thing, imo!

0002 looks like "dirty-hack", but reading closely I found no issues:
- SyncRepGetCandidatesStandbys syncs by spinlocks on every walsender
- all WalSndCtl->lsn increases monotonically under lock.

0004 it really doesn't cost anything and gives some value. So let it be.

We didn't measure things one-by-one, but I suppose 0001 and 0003 gives most
of gain. Still other thing are useful as well, I believe 0002 and 0004 are
worth to be committed.

I've rebased patches and simplified a bit 0001 and 0004:
- 0001 already uses static variable. So why don't just make it file-wide
and use in SyncRepWakeQueue directly?
- 0004 - there is no need in separate lsn and lsn_published. Lets simply
convert lsn to atomic variable.
And I've refactored condition under lock a bit to make it clear why test
against of just atomic WalSndCtl->lsn[mode] could be enough. It was a bit
non-obvious in the form it is in master branch.

And I've added 027_stream_regress_sync.pl in 0005 as tweaked copy of
027_stream_regress.pl to test synchronous replication under concurrent load.

--
regards
Yura Sokolov aka funny-falcon