diff --git a/notify-bench/.gitattributes b/notify-bench/.gitattributes new file mode 100644 index 0000000000000..d091436cac41a --- /dev/null +++ b/notify-bench/.gitattributes @@ -0,0 +1,5 @@ +# Stored diffs legitimately contain "space before tab" and trailing whitespace: +# every context line of a unified diff is a space followed by the original +# (tab-indented) source line. Exempt them, as the root .gitattributes does for +# other files containing special whitespace. +*.patch -whitespace diff --git a/notify-bench/README.md b/notify-bench/README.md new file mode 100644 index 0000000000000..93984f96e4534 --- /dev/null +++ b/notify-bench/README.md @@ -0,0 +1,172 @@ +# notify-bench — measuring the LISTEN/NOTIFY commit lock + +Fork-local benchmark harness for the cluster-wide heavyweight lock that +`PreCommit_Notify()` takes at `src/backend/commands/async.c:1309`: + +```c +LockSharedObject(DatabaseRelationId, InvalidOid, 0, AccessExclusiveLock); +``` + +Because `LockSharedObject()` builds the locktag with dbid `InvalidOid`, this lock is +cluster-wide (all databases). It is released at +`ResourceOwnerRelease(RESOURCE_RELEASE_LOCKS)` (`xact.c:2480`), i.e. **after** +`RecordTransactionCommit()` (`:2407`, containing `XLogFlush()` at `:1544` and +`SyncRepWaitForLSN()` at `:1599`) and **after** `ProcArrayEndTransaction()` (`:2431`). +Notifying transactions therefore cannot group-commit with one another. + +Context, full analysis and discussion: issue #82 in this fork. + +> **This directory is fork-local.** It is not proposed for upstream inclusion in this +> form. The instrumentation patch is a measurement device and is deliberately unsafe. + +## Headline result: the lock, isolated + +The problem with comparing `INSERT` against `INSERT; NOTIFY` is that the delta contains +the lock *plus* the SLRU queue insert, the utility statement, and an extra client round +trip. To isolate the lock, `instrumentation/0001-MEASUREMENT-ONLY-skip-notify-commit-lock.patch` +makes the identical binary skip **only** that one call when `PG_NOTIFY_NOLOCK` is set in +the postmaster environment. + +`synchronous_commit=on`, zero listeners, 3 reps of 10 s, median [min–max]: + +| clients | control | NOTIFY (lock) | NOTIFY (no lock) | gap recovered | +|--:|--:|--:|--:|--:| +| 1 | 824 [794–835] | 790 [618–844] | 733 [695–778] | — (no gap at c=1) | +| 4 | 2,059 [1946–2075] | 1,033 [934–1051] | 2,177 [2115–2300] | 111% | +| 16 | 7,811 [7430–8006] | 1,027 [911–1074] | 7,893 [7606–8290] | 101% | +| 64 | 16,307 [16044–16469] | 1,055 [979–1135] | 12,369 [11077–12973] | 74% | + +Lock-removal speedup: 2.1x (4 clients), 7.7x (16), 11.7x (64). Arm separation is +asserted by the harness: 72 `Lock`/`object` wait samples in the lock arm, **0** in the +no-lock arm. A run where that check fails to separate is void. + +This answers the open question from the 2025 pgsql-hackers thread — whether +`NotifyQueueLock` would simply become the next bottleneck. It does not: it is a short +LWLock around the SLRU insert and never spans the fsync. At 64 clients the residual +(NotifyQueueLock + SLRU + one extra round trip) is ~26% of the gap. + +### The mechanism, stated hardware-independently + +From `results/master.waits.csv`, both arms show exactly one backend in `IO`/`WalSync` at +any instant — one fsync in flight. The difference is who is queued behind it: + +| | `Lock`/`object` | `LWLock`/`WALWrite` | `IO`/`WalSync` | +|---|--:|--:|--:| +| NOTIFY, 32 clients | 610 | **0** | 18 | +| control, 32 clients | 0 | **373** | 18 | + +Control queues ~21 backends per in-flight fsync — group commit working. NOTIFY queues +**zero**, because everyone is stuck one step earlier on the heavyweight lock. +**Group-commit batch factor ~21 versus exactly 1.** Throughput ratios on any particular +disk are a consequence of that; this statement is the portable one. + +## Prototypes measured + +Two prototypes were built on top of the isolation result above. Both are in +`instrumentation/`, both are measurement devices rather than patch proposals. + +`0002-PROTOTYPE-move-notify-insertion-past-commit.patch` moves the queue insertion out of +`PreCommit_Notify()` into a new `PostCommitInsert_Notify()` called from +`CommitTransaction()` after `ProcArrayEndTransaction()`, serialized by a dedicated +`NotifyQueueInsertLock` LWLock. `synchronous_commit=on`, zero listeners, 3 reps, +median [min-max]: + +| clients | control | stock | v1 (heavyweight lock, pre-ProcArray) | v2 (LWLock, post-ProcArray) | no-lock | +|--:|--:|--:|--:|--:|--:| +| 4 | 2,059 | 1,033 | 2,130 | 2,129 [2109-2244] | 2,177 | +| 16 | 7,811 | 1,027 | 7,767 | 8,155 [7961-8176] | 7,893 | +| 64 | 16,307 | 1,055 | 10,965 | 16,507 [16402-16695] | 12,369 | + +15.7x over stock at 64 clients; commit latency 60.7 ms -> 3.9 ms, matching the +NOTIFY-free control. Both `Lock`/`object` and `LWLock`/`NotifyQueueInsert` sampled zero +waiters, so the insert lock is not observably contended. + +The v1 column is worth keeping: an earlier iteration that kept the heavyweight lock and +inserted *before* `ProcArrayEndTransaction()` reached only 10,965 at 64 clients. Moving +past the ProcArray removal and switching to an LWLock is worth the remaining ~50%, and +is also what makes the ordering invariant hold trivially instead of by lock tenure. + +Caveats specific to these runs: control cross-run variance is ~10% at 64 clients, so the +"v2 matches control" claim is shape, not precision; and with zero listeners **nothing ever +reads the queue**, so these runs cannot detect an ordering defect. See #85 for the +prototype's known gaps. + +## Scripts + +All run as an unprivileged user; PostgreSQL refuses to start as root. + +``` +scripts/writers.sh