Re: Reduce SyncRepLock contention on the commit path

22.09.2026 23:36, Vadim Ponomarev wrote:
> Hi Yura,
>
> Thanks for the numbers, backing the series and for the
> rebase.
>
> Both code suggestions are great.
>
> v3, on top of your v2, against 9e17d25e79d:
>
> 0001
> - SyncRepInitWakeList() asserts the list is empty. Nothing between
> filling it and draining it can throw today, so this only states
> the invariant.
> - Latches set front to back. Draining from the end reversed the
> release order. Harmless either way; my sense of beauty insisted.

Ok.
> 0004
> - Accessors renamed SyncRepGetLSN() and SyncRepSetLSN(). The setter
> asserts the lock.
> - Assert(MyProc->syncRepState == SYNC_REP_NOT_WAITING) moved above the
> lock-free exit, which skipped it.
> - Comment on lsn[] rewritten: 64-bit atomic read is not a plain
> load everywhere. Every access to lsn[] goes through the atomics API
> now, including the ones under the lock.

Ok.

> 0005 has one real problem. 'standby_1,standby_2' parses as FIRST 1,
> so remote_apply waits for standby_1 alone. The test dumps both
> standbys and compares them against the primary, taking no wait of its
> own. The setval() over pg_sequence right before the dumps is such a
> write. Put recovery_min_apply_delay = '10s' on standby_2 and it fails:
>
> ok 10 - compare primary and standby_1 dumps
> not ok 11 - compare primary and standby_2 dumps
>
> changed to ANY 2 (standby_1, standby_2) in v3 - remote_apply covers both.
> It also exercises the quorum path through SyncRepGetNthLatestSyncRecPtr(),
> which 0002 moves out from under the lock

Oh, thank you! That was the intention, but I failed to do it properly.

> One question in 0005: it runs the whole regression schedule a second
> time. Maybe would be behind PG_TEST_EXTRA?

Agree.

I think, it is ready for commit-fest to be created.

--
regards
Yura Sokolov aka funny-falcon