Limit concurrent backend WAL flushers - #2
Open
vbp1 wants to merge 3 commits into
Open
Conversation
Address review findings against the initial implementation: - Replace the hand-rolled 16-shard wait queue (spinlock + proclist + 10ms latch polling) with a single ConditionVariable. Wakeups are now event-driven: a released slot is handed over with ConditionVariableSignal(), which dequeues the process it wakes, and a process that leaves without taking the slot passes the signal on, so no wakeup can be lost and no thundering herd occurs. This also removes the O(waiters) proclist scans under a spinlock and the extra PGPROC fields; CV wait links are cleaned up at proc exit by the existing machinery. - Track the maximum requested flush LSN in one monotonically-advancing atomic (pg_atomic_monotonic_advance_u64) instead of per-shard values recomputed under a spinlock. The published value is clamped to the end of inserted WAL so that a corrupted page LSN cannot make every slot holder repeatedly request a flush past end of WAL. The clamp uses the end-position conversion of the insert position (GetXLogInsertEndRecPtr), matching what WaitXLogInsertionsToFinish() compares against: the start-position variant points past the page header whenever the insert position sits exactly on a page boundary, and benchmarking showed such requests tripping that function's "request to flush past end of generated WAL" complaint. - Defer waiter wakeups out of XLogWrite(): they now run after WALWriteLock is released, mirroring how walsender wakeups are handled. A slot is kept for the whole XLogFlush() loop instead of being released and re-acquired around LWLockAcquireOrWait(). - Handle injection points safely around critical sections. Plain INJECTION_POINT() could allocate inside one; worse, XLogFlush() is often entered with the caller (e.g. RecordTransactionCommit) already holding a critical section, so even a load placed before START_CRIT_SECTION is not always safe. Load the points only when no critical section is active, have the TAP test pre-load them in each session with injection_points_load(), and preload the module so that the wait callback's lazily-initialized state cannot allocate either. Both injection points now live in code reachable only by client backends, so background processes can no longer consume them and hang the test. - Report the wait with a new IPC wait event WalFlushLimit instead of reusing IO/WALSync, which made throttled backends indistinguishable from real fsync waits in pg_stat_activity. - Make the GUC PGC_SIGHUP: no shared memory is sized by it, and slot accounting stays balanced across reloads (config reload happens only between statements, so each XLogFlush call sees one value). Document that only client backends are subject to the limit. - Drop the guc.sql default-value check, which would break installcheck against a cluster with a non-default setting; the TAP test covers the GUC. - Extend the TAP test: a second injection scenario where both the slot handoff signal and the flush broadcast arrive while the waiter is registered but not yet sleeping (it must complete via the recheck after preparing to sleep), commit traffic across limit reloads bouncing between off, high and one (an acquire/release imbalance would wedge the slots and hang), and a crash-recovery check that commits made through the limited flush path are durable. Also verify that the server log stays free of flush-past-end complaints.
perf c2c profiling on a 4-socket stand (750 clients, limit 32) showed the limiter counters and the condition variable sharing one cache line with info_lck: the CV's internal spinlock alone accounted for 70% of that line's remote HITM traffic, competing with every XLogFlush entry, page-crossing insert, and hint-bit GetRedoRecPtr call. Give the counters and the condition variable cache lines of their own.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
wal_flush_backend_flushers(integer, default0= disabled, reloadable via SIGHUP): a cap on how many client backends may run the WAL write/fsync loop inXLogFlush()concurrently.With many hundreds of concurrently committing sessions, backends pile up on
WALWriteLock: the queue itself (lock acquire/release traffic, wait-list churn, spin delays) burns CPU and cache-coherency bandwidth without producing more flushed WAL, since the writes are serialized by the lock anyway. The limiter keeps the group-commit batching but replaces the crowd at the lock with a cheap sleep:pg_atomic_monotonic_advance_u64) and sleeps on aConditionVariable. Slot holders extend their flush requests to cover the published maximum, so waiters piggyback on the holders' fsyncs.ConditionVariableSignal()(the wakeup is bound to the dequeue, so it cannot be lost); a woken backend that leaves without taking the slot passes the signal on. Wakeups after the flushed position advances are broadcast afterWALWriteLockis released, mirroring how walsender wakeups are handled.IPC/WalFlushLimitinpg_stat_activity.GetXLogInsertEndRecPtr()), matching whatWaitXLogInsertionsToFinish()compares against, so a corrupted page LSN cannot poison every slot holder and the "request to flush past end of generated WAL" complaint cannot fire spuriously.perf c2cprofiling showed the counters and the CV's internal spinlock false-sharing a line withinfo_lck.Benchmark results
4-socket server (4 NUMA nodes, 240 CPUs), RAID WAL storage,
pgbench -c 750 -j 60 -T 900 --protocol=prepared, scale 75000, remote client over 100GbE,synchronous_commit=on,full_page_writes=on,wal_compression=lz4, build withNUM_XLOGINSERT_LOCKS=32. Clean data restore before each run; the baseline is the same binary with the feature disabled.Wait-event sampling (1 s, average waiting backends of 750):
LWLock/WALWritedrops from 125 to 10–20; the waiting moves into the passiveIPC/WalFlushLimitsleep. User CPU per transaction nearly halves (0.79 → 0.54 core-ms), run queue drops 163 → 124,Timeout/SpinDelaydrops 3x. The group-commit size stays the same (~73–76 commits per fsync): the gain comes from removing the contention overhead, not from batching changes. Per-commit durability wait is unchanged (0.93 ms vs 0.96 ms at baseline).Testing
src/test/recovery/t/055_wal_flush_backend_flushers.pl:ConditionVariablePrepareToSleeprecheck);limit=1;make check-worldgreen;pgindent/perltidyclean; docs (config.sgml) updated.The injection-point scenarios require
--enable-injection-pointsand preload the module (the wait callbacks fire inside critical sections); without the module the test skips those sections gracefully.