Re: Checkpointer write combining

On Wed, Aug 19, 2026 at 3:55 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> On Mon, Jul 20, 2026 at 6:21 PM Haibo Yan <tristan(dot)yim(at)gmail(dot)com> wrote:
> >
> > I have been looking through the v15 series.
>
> Thanks for taking a look.
>
> > The current implementation looks fairly complete for synchronous shared-buffer write combining, but I have a few questions about whether the API shape is general enough for the longer-term AIO and direct-I/O goals.
> > 1. Submission and completion lifetime
> > BufferWriteBatch is currently stack-allocated and contains pinned, content-locked buffer descriptors. FlushBufferBatch() then performs WAL flushing, checksum preparation, synchronous smgrwritev(), and I/O accounting, after which CompleteWriteBatchIO() marks the buffers clean.
> > That works well for synchronous writes, but it seems to encode an all-or-nothing synchronous lifetime.
>
> Reads use AIO and have an object in backend memory
> ReadBuffersOperation. When other backends complete a read, there are a
> limited number of things that they have to do in the shared callback
> and they can use a PgAIOHandle to do it. The issuing backend still
> needs access to information outside of what is in the PgAIOHandle (and
> has a different lifetime), and it gets that with the
> ReadBuffersOperation. Writes are similar with a few differences, but
> they will need a dedicated object for the issuing backend to keep
> track of the operations it initiates. In my latest posted version (v16
> [1]), I renamed BufferWriteBatch to WriteBuffersOperation to make it
> more symmetrical.
>
> > For an asynchronous implementation:
> >
> > * How would the batch object survive after submission?
>
> In the issuing backend which has responsibilities like adding the
> buffers to the writeback queue after completion.
>
> > * Would all pins and content locks remain held until completion?
>
> They have to be or someone could corrupt the buffer while it was being
> written out.
>
> > * Would submission, completion, and cleanup need to become separate phases?
>
> There will have to be three-ish functions -- one to start the writes
> in an issuing backend, one to complete the writes in either the
> issuing backend or another backend, and one to wait for the writes to
> be done and do any remaining tasks on the buffers.
>
> In v16 [1], I've renamed FlushBufferBatch() to WriteBuffers() and
> CompleteBufferBatchIO() to CompleteWriteBuffers(). With AIO, those
> will have to become StartWriteBuffers() and WaitWriteBuffers() with a
> separate completion callback implemented. I had an LLM demonstrate
> what AIO writes would look like on top of write combining. I looked at
> the commits it wrote and they have some problems and are missing some
> things (including not handling some architectural issues that Andres
> has already mentioned we have to solve before implementing AIO writes)
> -- but the broad strokes are correct and I think that is the shape
> we'll end up with [2]. Doing this exercise helped me figure out what
> to name functions and if I needed to change anything for
> future-proofing reasons.
>
> > 2. Failure and partial completion
> > The current path appears to assume that normal return from smgrwritev() means that the whole batch completed successfully, after which all buffers can be marked clean.
> > Would the future API need to represent:
> >
> > * partial completion,
> > * cancellation,
> > * submission failure,
> > * or an error affecting only part of a batch?
>
> Yes, it would but that code is non-trivial and not needed with a
> single combined write as smgrwritev() will complete or fail -- there
> won't be partial writes. I have some of the above in the hack-y AIO
> writes branch I mentioned and I don't think any of those
> considerations change the shape of the code in a way that merits
> changing synchronous combining.
>
> > Even if the current smgr contract remains all-or-nothing, it may be useful to make that contract explicit rather than embedding it in FlushBufferBatch().
>
> I don't know what you mean by this.

What I meant here was whether the all-or-nothing property was intended to be a
property of the WriteBuffersOperation abstraction, or was just a consequence
of the current synchronous smgrwritev() implementation. I was worried that
callers might start depending on “the whole operation completes together”,
making it harder later for one operation to contain multiple
independently completing AIO requests.

Your explanation of the future StartWriteBuffers() / completion callback /
WaitWriteBuffers() split answers that for me. In particular, I now understand
the current all-or-nothing behavior as a property of the synchronous
implementation, not something the operation abstraction is intended to
guarantee. So I don’t think anything needs to change here for this patch.

>
> > 3. Relationship with the read side
> > Write combining is important for direct I/O, but the same applies to reads. Issuing one 8 kB direct-I/O request per buffer would also lose the readahead and aggregation normally provided by the kernel page cache.
> > I do not think the read and write buffer state machines necessarily need to share the same API, since their locking and completion rules are quite different.
> > However, could the lower-level physical-I/O parts be read/write neutral?
> > For example:
> >
> > * contiguous extent description,
> > * io_combine_limit,
> > * smgr segment and boundary handling,
> > * multi-block submission,
> > * completion representation,
> > * and physical-I/O accounting.
>
> io_combine_limit is already what is used for writes in my code. I had
> a development-only guc letting me control it separately but I got rid
> of that in v16. The boundary handling can't work the same way as reads
> because reads only read ahead and don't consider going backwards to
> create a batch (wouldn't make sense). And for buffer access strategy,
> it works differently as well with writes. The completion callbacks
> will look similar but can't be shared because you have different
> things you need to do on completion. All of the low-level IO stuff
> will be shared, but at the bufmgr layer, not much issuing/completing
> code can be shared, as it's not the same behavior (flushing WAL,
> locks, fsync requests, writeback requests, etc). And I/O accounting at
> bufmgr level has always been done with separate traces and pg_stat_io
> function calls for reads and writes.
>
> > Those pieces seem closely related to what the recent ReadStream work and the broader AIO effort already need
> > Otherwise, is there a risk that the write-batching code and the read-side AIO infrastructure evolve into two parallel mechanisms for essentially the same physical-I/O aggregation problem?
>
> I have made an effort to make the structures and names more consistent
> with the Read concepts in v16 -- including moving from
> BufferDescriptors to Buffers in the WriteBuffersOperation.
>
> > 4. Foreground backend work and lock duration
> > For foreground backends, combining a required victim with several adjacent buffers increases more than just the I/O size; it can introduce unpredictable latency spikes for user queries. It also increases:
> >
> > * the amount of foreground cleanup work,
> > * the number of pinned buffers,
> > * and the time for which content locks may be held.
> >
> > Should foreground batching therefore have a separate latency or work budget, in addition to the pin limit and maximum combine size?
>
> Number of pinned buffers is already bounded by io_combine_limit and
> eventually by effective_io_concurrency -- like reads. I think that is
> sufficient with only write combining.
> Once we have AIO writes, there will be much more competition for
> pinned buffers (since we have reads already), so perhaps we will need
> to think about the pinned buffers limits, but I don't foresee any
> issue with non-toy shared buffers with just write combining.
>
> As for foreground cleanup work and content lock duration, if there is
> enough shared buffer pressure that you are evicting buffers at all
> (which is the only time you'll do this combining for regular
> backends), then the buffers you are eagerly cleaning will need to be
> cleaned by someone. Combining won't create new writes, only change how
> they're issued. And if there isn't shared buffer pressure, it is less
> likely that you are I/O bound. However, there is a possible negative
> case here I describe in my most recent performance analysis here [1]
> (see the section entitled "Mixed buffer hits leading to non-contiguous
> rings").
>
> If your point is that a 128kB write takes longer than an 8kB write,
> sure, that's usually true (though if you flood the device with enough
> small requests, you may end up having the 8kB write take longer).
> However, the aggregate time spent holding locks will go down with
> write combining. If you write out 16 8k blocks individually, each one
> pays the full per-write cost -- making the aggregate time spent
> holding locks much higher than writing a single 128k write.
>
> Now if all the concurrently working backends are contending for the
> lock on the buffers that are being written out in that 128k write, you
> can see p999 latency spikes occasionally -- and I did see that in a
> scenario I concocted to exercise this.
>
> But the more common case is that in IOPs constrained environments, the
> p99 latency with combining can improve by 80%+. The writes being done
> by regular backends and background writer are random IO without
> combining, so making them more sequential relieves a lot of IOPs
> pressure.
>
> Separately, there is a cost to doing a few more buffer table lookups,
> but I did performance evaluation and, even in the worst case, it seems
> to have basically no impact given the cost of writing.
>
> I think once we can issue more than one combined IO (with AIO) then we
> have to be way more careful of this stuff. Backends absolutely can't
> wait for multiple IOs to finish when their victim was in the first IO
> -- even if the IOs are overlapped with AIO. And we'll have to be
> worried about putting too much IO in flight.

I did some more testing on this point.

One additional foreground-latency case I was wondering about was the WAL side
of eager combining. In GatherContiguousDirtyBuffers(), once the required
victim itself needs a WAL flush, allow_pending_wal allows a newer adjacent
dirty buffer into the batch. That buffer can raise
WriteBuffersOperation.max_lsn, so the eventual XLogFlush() target can be
substantially ahead of what the required victim alone needed.

I instrumented this and exercised it through the real GetVictimBuffer() ->
ClaimVictimBuffer() -> WriteBufferAndNeighbors() path under concurrent pgbench
workloads. The mechanism is actually fairly common: about 5.6–28% of
dirty-victim writes in the representative workloads, and 53–63% in an
intentionally adversarial workload, had max_lsn > required_lsn.

But I couldn’t find a measurable cost from it. In 99.9–100% of the sampled
amplified cases, the flush pointer had already advanced through max_lsn by the
time XLogFlush() was called, due to concurrent WAL activity, so the call did
no real additional flush work. I also saw no consistent difference in p99/p999
transaction latency or TPS when comparing the current behavior with a
diagnostic version that rejected neighbors with LSNs beyond the required
victim’s LSN.

This was all on fast local NVMe, so slower or heavily contended WAL storage
could behave differently, but based on these results I agree that adding
another foreground work/latency limit for synchronous combining doesn’t seem
justified.

One small wording point I noticed while doing this: the allow_pending_wal
comment says that admitting such neighbors “adds no WAL flushes”. That’s true
in terms of the number of XLogFlush() calls, but it can increase the target
LSN of that call. It didn’t matter in my tests, but perhaps “adds no
additional XLogFlush() calls” would make the distinction a little clearer.

>
> > My main concern is that, without separating these layers now, asynchronous writes may require another substantial API refactoring later.
>
> Yes, we want to avoid that, but I think with the latest revisions, we
> are being as careful as we can reasonably be.
>
> - Melanie
>
> [1] https://www.postgresql.org/message-id/CAAKRu_Z%2BGEgkHqQPajsy0ApDWYTvfmR5G6AjnE7mVcy1_00kxQ%40mail.gmail.com
> [2] https://github.com/melanieplageman/postgres/tree/aio_writes_llm-draft
> (I will likely delete this branch later, sorry to anyone looking at
> this in the hackers archive)

Thanks,
Haibo