Re: [PATCH] Batched clock sweep to reduce cross-socket atomic contention

From: "Greg Burd" <greg(at)burd(dot)me>
To: "Andres Freund" <andres(at)anarazel(dot)de>
Cc: "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Tomas Vondra" <tomas(at)vondra(dot)me>, "Nathan Bossart" <nathandbossart(at)gmail(dot)com>
Subject: Re: [PATCH] Batched clock sweep to reduce cross-socket atomic contention
Date: 2026-07-25 10:45:26
Message-ID: 4552df91-98bc-40e8-9189-a167daab6f0d@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Wed, Jul 22, 2026, at 11:10 AM, Andres Freund wrote:
> Hi,

Hi Andres,

You and Ants landed on the same core correction, so I've owned the
StrategyRejectBuffer / BAS_BULKWRITE mistake in my reply to him rather than
repeat it here -- short version: you're both right, BAS_BULKWRITE keeps the
writes on the backend, there is no hand-off, and I've dropped that framing.
Let me answer the points that were specific to your mail.

> On 2026-07-14 13:53:06 -0400, Greg Burd wrote:
>> The buffer cache ring strategy didn't only bound cache pollution, it also
>> deferred writeback. A backend running a big COPY, bulk UPDATE or VACUUM
>> reused a small ring and, via StrategyRejectBuffer, pushed dirty victims back
>> for the bgwriter and checkpointer to write instead of writing (and
>> WAL-flushing) them inline on its own allocation path. Pull the rings (as in
>> 0003) and, done naively, that deferral goes too.
>
> That's not right, StrategyRejectBuffer does that only for a bulkread
> strat. And because of the limited number of buffers used for bulkwrite, they
> don't actually tend to involve bgwriter a lot.
>
> The effect is literally the opposite for bulk copies. Without the strategy
> there's basically no back pressure against a backend dirtying a lot during
> bulk writes, with the strategy there's a *lot* (perhaps too much)
> backpressure.
>
> There is no such handing off.

Right, and this reframes the whole thing for me. I had been treating the
ring's writeback behavior as a cost to preserve; it's actually backpressure to
preserve. I confirmed the "no backpressure without it" direction directly: a
sustained bulk-dirtying workload with the bgwriter pushed as hard as it goes
still ends with the backend doing ~100% of the relation writes itself -- the
bgwriter just falls behind and every allocation flushes inline. So removing
the rings removes real backpressure and "make the bgwriter keep up" does not
replace it.

The consequence is that I'm dropping the BufferAccessStrategy removal (old
0003) from what I'm putting forward. It needs a genuine replacement for the
bulk-writer backpressure BAS_BULKWRITE provides, which is its own project, and
your "perhaps too much" aside is interesting on its own -- if the current
backpressure is heavier than it needs to be, the right move might be to tune
or rethink it rather than either keep it as-is or delete it. But that's not
this patch. What I'm left proposing is two independent things: the batched
sweep, and the cooling-stage evictor with the rings left in place.

>> CREATE TABLE bulk (id int, pad text) WITH (fillfactor=90);
>> INSERT INTO bulk SELECT g, repeat('x',180) FROM generate_series(1,2e7) g;
>> VACUUM (FREEZE) bulk; CHECKPOINT;
>>
>> -- (1) UPDATE, dirties every page
>> UPDATE bulk SET id = id + 1;
>>
>> -- (2) bulk INSERT into an empty table
>> CREATE TABLE ins (id int, pad text);
>> INSERT INTO ins SELECT g, repeat('y',180) FROM generate_series(1,2e7) g;
>>
>> -- (3) VACUUM after dirtying half the pages
>> UPDATE bulk SET pad = repeat('z',180) WHERE id % 2 = 0; CHECKPOINT;
>> VACUUM bulk;
>>
>> One caveat up front: (2) is INSERT...SELECT, not the COPY command.
>
> INSERT ... SELECT does not use the bulkwrite strategy, so there can't be an
> effect from removing the strategies until the VACUUM.
>
>
>> It hits the same relation-extend + WAL-logged-write path BAS_BULKWRITE
>> served, which is what I wanted to stress, but if that distinction matters to
>> anyone's conclusion I'll re-run with COPY FROM.
>
> It's not at all the "same relation-extend + WAL-logged-write path". INSERT
> SELECT uses individual heap_inserts, COPY uses heap_multi_insert. The latter
> extends does a lot less WAL logging and extends the relation much more
> aggressively (heap_insert() only bulk extends if there's contention).

RelationGetBufferForTuple/RelationAddBlocks extends by num_pages scaled by the
extension-lock waiter count, so a single-tuple heap_insert extends one page
unless it's contended, while heap_multi_insert asks for many; and
heap_multi_insert emits one XLOG_HEAP2_MULTI_INSERT per batch versus one record
per row. So (2) in my script was not exercising the BAS_BULKWRITE path at all,
and it was not the "same" write/extend path as COPY. Both criticisms are
correct and the workload was mislabeled.

>> For each statement I record time_s (wall seconds), bwrites (buffers the
>> backend wrote itself -- sum(writes) from pg_stat_io where object='relation'
>> after a forced flush, i.e. the writes the rings used to hand off), and
>> wal_gb (LSN delta, just a check the two builds did the same work). Full
>> series, Option B included:
>>
>> workload metric stock patched impact of patches
>> ------------ --------- ---------- ---------- ------------------
>> bulk UPDATE time_s 26.0 27.3 5% slower
>> bwrites 90754 158165 74% more
>> wal_gb 5.83 5.90 +1%
>> bulk INSERT time_s 14.2 15.6 10% slower
>> bwrites 0 0 none; extend path
>> wal_gb 5.00 5.11 +2%
>> VACUUM time_s 11.4 9.3 19% faster
>> bwrites 996255 390945 61% fewer
>> wal_gb 6.80 6.77 -1%
>
> Af course VACUUM is faster if you allow it to fill up all of shared buffers
> with dirty buffers. FWIW, you can get that effect today using
> VACUUM (BUFFER_USAGE_LIMIT 0)

Yes. I re-ran this and you're right: the VACUUM "win" is just the removal of
the BAS_VACUUM self-throttle, not anything my change does. With a corrected
run the patched VACUUM time is ~the same as stock, and stock with
BUFFER_USAGE_LIMIT 0 gets essentially the same effect. I've withdrawn the
VACUUM-speedup claim; it was measuring the throttle, not the evictor.

> Since a bulk update doesn't use a strategy, I don't see how strategy related
> changes can trigger 74% more writes via bgwriter.

They can't, and that was the tell that my explanation was wrong. The +74%
was not a strategy effect -- a bulk UPDATE uses no strategy in either build.
Isolating it (benchmarking 0001+0002 with the rings still present, which is
also the test Ants suggested) shows the write-shifting comes from the cooling
change in 0002 -- specifically an over-eager bgwriter tweak I had folded in
there ("Option B") that pushed writes off the backend onto the bgwriter. That
was an eviction-path change riding in 0002, not ring removal, and given the
backpressure discussion above it's the wrong behavior anyway, so I'm removing
it. Once it's out, the bulk-UPDATE delta between stock and the cooling evictor
sits inside run-to-run noise; I won't be presenting it as a win or a
regression.

> Greetings,
>
> Andres Freund

Net: the honest residue of that whole benchmark is "I mismeasured." Where I
think there is a real, defensible result is on the *cost of the 0..5
usage_count itself* under a hot, continuously-evicting large pool with
huge_pages off -- the clock hand doing ~5*NBuffers dTLB-missing descriptor
visits per victim because it decrements a hot buffer five times and resets its
scan counter on every decrement. I've been measuring that (ticks-per-victim,
and the residency-fraction regime where it bites) and will bring it as its own
thread with a methodology built to your "real IO, not all-in-page-cache,
measure which buffers get evicted" bar rather than tacking it onto this one.

Thanks for the corrections and insights.

best.

-greg

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andreas Karlsson 2026-07-25 11:56:29 Re: Add cleanup parsing contexts for pg_hba and pg_ident files
Previous Message Greg Burd 2026-07-25 10:35:44 Re: [PATCH] Batched clock sweep to reduce cross-socket atomic contention