Use streaming read I/O when enabling data checksums online

On 3 Jun 2026, at 13:10, cca5507 <cca5507@qq.com> wrote:

Attach a simple patch for $subject. I think we prefer streaming IO instead of
ReadBufferExtended(). Do I miss something?

Thanks! This code was written well before read streams existed, and recent
work was focused on correctness of operation, that's the main reason it's using
ReadBufferExtended still. We probably want do something like this once the
tree opens up after the v19 feature freeze.

--
Daniel Gustafsson

 Attach a simple patch for $subject. I think we prefer streaming IO instead of
 ReadBufferExtended(). Do I miss something?

Thanks!  This code was written well before read streams existed, and recent
work was focused on correctness of operation, that's the main reason it's using
ReadBufferExtended still.  We probably want do something like this once the
tree opens up after the v19 feature freeze.

Get it! I created a CF entry for this:

https://commitfest.postgresql.org/patch/6841/

--
Regards,
ChangAo Chen

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: tested, passed

Hi,

Contents & Purpose
==================
This patch converts ProcessSingleRelationFork() in
src/backend/postmaster/datachecksum_state.c from a plain per-block
ReadBufferExtended() loop to the new ReadStream API.
This is a pure internals/performance change: the external contract is
unchanged.

No documentation changes are included, which seems right since no
user-visible behavior changes.

Initial Run
===========
The patch applies cleanly to HEAD and builds without warnings.

I built it and ran it end-to-end by hand: initialized a cluster
without checksums, created a ~200,000-row/5,883-page table, and called
pg_enable_data_checksums(). Progress was visible via
pg_stat_progress_data_checksums while the new stream-based scan was
running, and data_checksums correctly flipped to "on" on completion.
I then independently verified correctness with the offline pg_checksums
--check tool: 9,434 blocks scanned across the cluster, 0 bad checksums.

I also exercised the abort path (the "success = false; break;" case
when checksums are disabled while an enable is in progress): I re-ran
pg_enable_data_checksums() with a cost_delay/cost_limit slow enough to
catch it mid-scan, then called pg_disable_data_checksums()
concurrently. The worker logged "data checksums processing was aborted"
and cleanly transitioned to disabled, and the server stayed healthy
afterward.

Performance
===========
I benchmarked this against pre-patch HEAD using a ~7.5 GiB table
(956,815 blocks) with debug_io_direct=data to force real synchronous
reads, 5 repetitions per configuration, median reported:

- io_method=sync:
HEAD = 460003ms, 958482 reads, 299082ms io_time
patch = 383069ms, 60248 reads, 264387ms io_time

- io_method=worker:
HEAD = 458885ms, 958482 reads
patch = 352155ms, 60248 reads

There's a ~16x drop in read syscalls (matching the default
io_combine_limit). My test storage is SSD, so the clock time multiplier
may understate the win on higher-latency disks.

Nitpicking & Conclusion
=======================
One nit: the new "BlockRangeReadStreamPrivate p;" declaration is not
properly indented.

Please run pgindent and then I'll be +1 for committer review.

Best regards,
Enrique.

One nit: the new "BlockRangeReadStreamPrivate p;" declaration is not
properly indented.

Please run pgindent and then I'll be +1 for committer review.

Fixed, thanks!

--
Regards,
ChangAo Chen

Hi ChangAo, for some reason the commit fest entry and the thread in
psql-hackers is not showing your last email with the fixed patch.

I've changed the status to "Ready for Committer" anyway and hope that it'll
be shown soon.

Best regards,
Enrique.

El dom, 12 jul 2026 a las 18:06, cca5507 (<cca5507@qq.com>) escribió:

On 12 Jul 2026, at 18:06, cca5507 <cca5507@qq.com> wrote:

One nit: the new "BlockRangeReadStreamPrivate p;" declaration is not
properly indented.

Please run pgindent and then I'll be +1 for committer review.

Fixed, thanks!

Thanks for this work, I have it on my radar to review and commit for v20, but
will wait for the work of cleaning up and fixing the online checksums code
which is currrently ongoing for the beta period.

--
Daniel Gustafsson

Hi ChangAo, Daniel, Enrique,

On Mon, Jul 13, 2026 at 9:50 AM Daniel Gustafsson wrote:

Thanks for this work, I have it on my radar to review and commit for v20,
but will wait for the work of cleaning up and fixing the online checksums
code which is currrently ongoing for the beta period.

I read v2 against the current online-checksum code and the earlier design
thread. I did not find a correctness problem in the conversion.
In particular, the fixed numblocks boundary is preserved, and ending the
stream on abort releases the buffers read ahead by the stream.

The main benefit of moving this path to ReadStream is that it can use the
future direct I/O and asynchronous I/O infrastructure without double
buffering. Enrique's results also show an immediate reduction in system
calls and execution time. It would also be interesting, though not a
prerequisite for moving the patch forward, to measure foreground throughput
and latency while checksum enable runs with a nonzero cost_delay and a fairly
low cost_limit. Limiting its effect on a running workload is the reason
those parameters exist.

The other useful opposite case would be a database with many empty tables
and indexes. v2 creates a stream for every existing fork,
while the current loop has almost no work for an empty fork. Perhaps a
numblocks == 0 fast path is worthwhile regardless, but a short measurement
would tell us whether this matters.

Thank you!

Best regards, Andrey Borodin.

Hi Andrey, all,

On Mon, Aug 24, 2026 at 8:32 PM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

Hi ChangAo, Daniel, Enrique,

On Mon, Jul 13, 2026 at 9:50 AM Daniel Gustafsson wrote:

Thanks for this work, I have it on my radar to review and commit for v20,
but will wait for the work of cleaning up and fixing the online checksums
code which is currrently ongoing for the beta period.

I read v2 against the current online-checksum code and the earlier design
thread. I did not find a correctness problem in the conversion.
In particular, the fixed numblocks boundary is preserved, and ending the
stream on abort releases the buffers read ahead by the stream.

The main benefit of moving this path to ReadStream is that it can use the
future direct I/O and asynchronous I/O infrastructure without double
buffering. Enrique's results also show an immediate reduction in system
calls and execution time.

I haven't looked into the details of this patch. It seems also helpful
to conduct some benchmarking for this patch on a simulated
high-latency storage as suggested by Andres [1]/messages/by-id/dmf5ladi2amq656myv7zjl4pj4u3v2cp3azteliauifxizljej@bmwabkp5hdpi. It helps to tell the
story of performance winning more vividly.

[1]: /messages/by-id/dmf5ladi2amq656myv7zjl4pj4u3v2cp3azteliauifxizljej@bmwabkp5hdpi

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

Hi,

On Mon, 24 Aug 2026 at 15:32, Andrey Borodin <x4mmm@yandex-team.ru> wrote:

Hi ChangAo, Daniel, Enrique,

On Mon, Jul 13, 2026 at 9:50 AM Daniel Gustafsson wrote:

Thanks for this work, I have it on my radar to review and commit for v20,
but will wait for the work of cleaning up and fixing the online checksums
code which is currrently ongoing for the beta period.

I read v2 against the current online-checksum code and the earlier design
thread. I did not find a correctness problem in the conversion.

I wanted to point out a couple of things. I am not sure whether either
is a problem, but I think they are worth mentioning. Before this
patch, an abort caused the function to return immediately after
processing the current buffer. With the read stream, read_stream_end()
stops further look-ahead, but still waits for and releases buffers
already queued by the stream.

1. Performance point. I don't think this is a problem since there
won't be many buffers.

2. Could this cause an error from reading a block that would not
otherwise have been read because of the abort?

--
Regards,
Nazir Bilal Yavuz
Microsoft

Hi Nazir,

On Mon, Aug 24, 2026 at 6:13 PM Nazir Bilal Yavuz wrote:

2. Could this cause an error from reading a block that would not
otherwise have been read because of the abort?

Yes. read_stream_end() drains queued reads, so WaitReadBuffers() can
report an error from a prefetched block after the abort request.

The relation is locked against truncation and the callback is bounded by
the initial numblocks, so this can only expose an actual I/O error for a
valid relation block, not an out-of-range read.
Thank you for pointing this aspect.

Best regards, Andrey Borodin.