| From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
|---|---|
| To: | Haibo Yan <tristan(dot)yim(at)gmail(dot)com> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de> |
| Subject: | Re: Checkpointer write combining |
| Date: | 2026-08-19 22:54:55 |
| Message-ID: | CAAKRu_b=roA_nntHxV3POk06b0JPhT5N3HDDSNRBV+2FgLcUBQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
> 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.
> 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)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Geoghegan | 2026-08-19 22:59:21 | GIN page deletion and page recycling bugs |
| Previous Message | Peter Smith | 2026-08-19 22:47:03 | Re: Support EXCEPT for TABLES IN SCHEMA publications |