Reduce SyncRepLock contention on the commit path by pg-hub-mirror[bot] · Pull Request #409 · MisterRaindrop/postgresql-github-mirror

pg-hub-mirror · 2026-09-17T19:54:35Z

Read-only mirror. Reply and review on pgsql-hackers; activity here is not sent upstream.

Original author: Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>
Mailing list: pgsql-hackers
Message-ID: 3024af42-d264-4bb8-aa58-377b286bd4b3@postgrespro.ru
Original email

Patch files:

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 2e57790 ("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 7e6e294.

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

SyncRepWakeQueue() sets each released backend's latch while holding SyncRepLock exclusively. A latch is a kill() syscall whenever its proc is asleep, and at a high commit rate the walsender runs one of them per released commit inside the very section every committer lines up on. ProcArrayGroupClearXid() already wakes its batch only after ProcArrayLock is down, for the same reason. Collect the released procs into a list instead, and set their latches once the lock is released. The unlink, the write barrier and the state store stay under the lock: a waiter reads syncRepState without the lock and must never find itself completed while still on the queue. Nothing in the deferred loop can error out, so a released proc cannot be left completed but unlatched short of the process dying outright -- a window the in-lock SetLatch had as well. A proc that noticed its state on its own and moved on, even into a new wait, gets a spurious latch set, which every latch sleeper tolerates. The list is sized to MaxBackends and allocated once per releasing process. A proc waits in at most one queue, so one list of that size bounds a walk over all three.

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.

ProcessStandbyReplyMessage() calls SyncRepReleaseWaiters() for every reply it processes, and several replies routinely sit in the walsender's socket together. Each of those calls takes SyncRepLock exclusively, so a batch of replies costs the committers one period of that lock apiece -- computed, for all but the last reply, from positions the next message in the same batch immediately makes stale. Have a reply only mark a release as pending, and run one release at the end of the drain. The positions in shared memory are the newest of the batch by then, so the single pass releases everything the individual passes would have. A deferred release must survive every way out of the drain, because the positions the reply already stored are valid whatever follows and the committers it acknowledged have no other process to wake them: - the standby's goodbye, an EOF, an invalid message type and an unexpected message type each run the pending release before leaving. A clean standby shutdown sends its final reply and the goodbye back to back, which makes that exit the routine one rather than the exotic one. - an error thrown while a later message in the same drain is parsed -- a torn message above all -- leaves through WalSndErrorCleanup(), which runs the pending release after the locks are dropped. The test makes both coincidences certain instead of likely. For the first, a paused standby holds a remote_apply committer in the queue, the walsender is held with SIGSTOP while the standby applies past the commit and shuts down, and the released walsender drains the final reply and the goodbye in one pass. For the second, an injection point right after a drained reply stands in for the torn message; it fires on every reply, so the walreceiver is held with SIGSTOP while replay proceeds from WAL already on standby disk, which makes the first reply after release the one carrying the apply position the committer waits for. Both halves fail without their fix.

…lock SyncRepWaitForLSN() runs on every commit that wrote WAL, and it takes SyncRepLock exclusively before it can find out whether there is anything to wait for. On a busy primary a large share of those commits find their LSN already acknowledged and queue for nothing, so the answer costs them a period of the lock every other committer is lining up on. Turn lsn[] into an atomic watermark, and read it before taking the lock. A watermark that already covers the commit's LSN says a valid quorum acknowledged it, which is exactly the answer the check under the lock gives. It is only ever moved forward, so a read gone stale can send a committer to the slow path that would have exited, but never past a wait it owes. On platforms where pg_atomic_read_u64() is not a plain load the read is itself a compare-and-exchange, or a spinlock acquisition where 64-bit atomics are emulated. Whether the exit still pays for itself there is untested.

It will allow to experiment with synchronous replication code more wildly. 027_stream_regress_sync.pl is a copy of 027_stream_regress.pl with couple of changes: - synchronous_commit is set to remote_apply - second standby added and checked - both standbys are in synchronous_standby_names - rely on remote_apply for standbys to catchup.

vbp1 and others added 5 commits September 17, 2026 19:54

pg-hub-mirror Bot locked and limited conversation to collaborators Sep 17, 2026

Navigation Menu

Reduce SyncRepLock contention on the commit path - #409

Reduce SyncRepLock contention on the commit path#409
pg-hub-mirror[bot] wants to merge 5 commits into
masterfrom
pg-hub/mirror-patch-49469f0e1c2f1e1d

pg-hub-mirror Bot commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pg-hub-mirror Bot commented Sep 17, 2026

Prior work

Measurements

Testing

Notes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants