Hi, Apologies for the delay.
TL;DR:
- The speedup from 0005 depends a lot on how much the incremental backup
has changed
since the last one: ~9.8x at 14% delta, ~3.7x at 33%, ~1.2x at 57%. That
explains the
difference between Jakub's 9.4x speedup and the ~1.5x I first saw.
- Most of the gain comes from avoiding the kernel's own readahead on
scattered blocks
(it was doing more than double the I/O needed), not from concurrency.
- Merging adjacent blocks matters. Without it, 0005 is slower than master
on backups
with a high delta.
- maintenance_io_concurrency (m_io_c) beyond 2 only helps a little, and
only when
there are many adjacent blocks. m_io_c = 1 is still worse though.
- Code changes in this version: prefetch is enabled again for m_io_c = 1,
the merge
limit is now io_combine_limit, there is a new limit on how far ahead we
prefetch,
and the prefetch state moved into a new IncrPrefetch struct.
> I've verified ~9.4x improvement with 0005-incremental-backup
I was confused by the big difference between our results, since I had
initially seen
only ~1.5x. After replicating your tests I found the reason: the speedup
varies a lot
depending on how much delta your incremental backup has.
I followed the same method you described:
initdb # summarize_wal=on, maintenance_io_concurrency=2, ...
pgbench -i -s 500 --fillfactor=100 -d dbname=test # ~7.5GB
pg_basebackup -D /pg-test/full -c fast
# ~14% of data change
psql test -c "CHECKPOINT"
./measure pg_basebackup --incremental /pg-test/full/backup_manifest
--wal-method none --target server-blackhole --checkpoint fast
# "measure" is a simple script that runs the command 3 times, dropping
caches in between
That gives an incremental backup of ~1.1G (~14% delta). To increase the
delta I then ran
pgbench + checkpoint a few more times between backups. Results (in seconds):
delta change without 0005 with 0005 speedup
------------- ------------ --------- -------
~14% (~1.1G) 42.48 4.31 ~9.8x
~33% (~2.5G) 27.56 7.49 ~3.7x
~57% (~4.3G) 15.60 13.13 ~1.2x
So the higher the delta, the smaller the gain from 0005. I think this ties
back to the
kernel behavior you observed.
Reading scattered blocks the way pg_basebackup does today seems to confuse
the kernel.
After a single block read, the kernel may readahead the following blocks,
expecting a
sequential pattern. With scattered delta blocks that never happens, so the
prefetched
data is wasted. I actually checked this on /proc/<pid>/io during a backup
of ~14% delta:
Without 0005
```
rchar: 1139995913 # bytes requested by the process (1.1GB)
syscr: 138285
read_bytes: 2557988864 # bytes actually read from storage (2.5GB)
```
With 0005
```
rchar: 1139966637 # bytes requested by the process (1.1GB)
syscr: 138279
read_bytes: 1143574528 # bytes actually read from storage (1.1GB)
```
So, without 0005 the kernel does more than double the I/O we need. 0005
helps with
concurrency, but its major improvement is avoiding this misguided readahead.
This explains the results above. With 14% delta from pgbench, most blocks
are
scattered, so 0005 avoids all that double I/O and gets a huge speedup. At
57% the delta
blocks are much more adjacent, so the kernel's own readahead gets a good
part of its
guesses right and 0005 has less to improve on.
A few other questions I tried to answer in this round:
1. Does increasing maintenance_io_concurrency make a big difference?
Not really. There's some gain up to a point, but it's small. This also
reinforces that
most of the gain of 0005 comes from avoiding the kernel's double I/O,
not from
concurrency itself. Below are some results of backup with 14% delta (in
seconds):
with m_io_c = 2: 4.45
with m_io_c = 16: 4.14
with m_io_c = 32: 4.07
with m_io_c = 64: 4.18
This made me wonder if we should stop making the prefetch window tied to
m_io_c and
instead just use a small fixed number like 2. But a higher m_io_c does
give a
modest gain on backups with a higher delta (more adjacent blocks). With
~57% delta:
with m_io_c = 2: 13.13
with m_io_c = 16: 10.28
2. Does maintenance_io_concurrency = 1 give any gain?
Yes, but less than 2. With ~14% delta (scattered blocks), in seconds:
with m_io_c = 2: 4.31
with m_io_c = 1: 6.94
3. Is merging adjacent blocks really helping?
At some point I considered dropping the merge-of-adjacent-blocks piece,
so I decided
to measure it on a 57% delta backup with m_io_c = 16 (results in
seconds):
without 0005: 15.60
0005 with merge: 10.28
0005 without merge: 20.41
It helps a lot. Without it, 0005 makes incremental backups with a high
delta
slower than not having 0005 at all (15.60 above).
My environment:
AWS EC2 c3.8xlarge; EBS vol 22000 IOPS, 1500MB/s max throughput;
RHEL 9.8; Kernel 5.14.0-687.41.1.el9_8.x86_64
Now regarding the code changes of this last iteration:
1. > it seems maintenance_io_concurrency=1 disables prefetch?
Yes, I had disabled it, thinking it wouldn't help. After the tests above
I enabled
it again.
2. We had previously agreed on a limit for how many blocks can be merged
into a single
fadvise. I was using maintenance_io_concurrency for that, but I thought
io_combine_limit might be a better fit according to its description. Do
you think
that makes sense?
3. The previous version had no limit on how far ahead, in blocks, we could
prefetch.
Each iteration runs one prefetch_next_incremental_run(), which can cover
many
blocks when they're adjacent. With lots of adjacent blocks we could get
far ahead of
the block being read, filling the page cache with pages that won't be
needed any
time soon and might even be evicted before they are read.
Based on that, I added a distance limit, in blocks, of
maintenance_io_concurrency * io_combine_limit. When there are many
adjacent blocks,
that keeps us at most maintenance_io_concurrency fadvise calls ahead of
the current
block.
4. I added a new struct named IncrPrefetch to the code to hold some
parameters of the
prefetch mechanism. I thought sendFile() was getting a lot of new
variables and
becoming harder to read. I hope that makes sense.
BTW thank you very much for helping with the reviews, I appreciate it.
Regards,
Gustavo Oliveira
Em qui., 15 de out. de 2026 às 09:08, Heikki Linnakangas <[email protected]>
escreveu:
> On 07/10/2026 12:55, Jakub Wartak wrote:
> > On Tue, Oct 6, 2026 at 5:42 PM Heikki Linnakangas <[email protected]>
> wrote:
> >
> >> I think that comment is wrong. A short read is possible for other
> >> reasons than a truncated file, e.g. if the read is interrupted by a
> >> signal. In the past, we've made that same assumption that you never get
> >> a short read on a BLCKSZ-sized read in other places too, but it was
> >> always questionable. Thomas Munro fixed it for smgrread() in commit
> >> 4908c58720.
> >>
> >> I think we should now fix that in read_file_data_into_buffer(), too.
> >> Make it retry, so that it never returns fewer bytes than requested
> >> except for EOF. With a larger buffer, a short read becomes more likely.
> >
> > OK, I see, thanks, so I've tried this in the attached 0004.
>
> Ok, looks good as far as it goes. But looking at basebackup_read_file(),
> I think there's a race condition with partial_read_ok == false. This
> could happen:
>
> 0. read_file_data_into_buffer() observes a checksum failure thanks to a
> torn write, and retries reading a single BLCKSZ block.
> 1. basebackup_read_file() is called with partial_read_ok == false,
> nbytes=BLCKSZ.
> 2. It reads the first 4096 bytes of the block. (e.g. because of
> SIMULATE_SHORT_READ, or EINTR)
> 3. The file is truncated
> 4. basebackup_read_file() makes another pg_pread() call to read the rest
> of the block. It returns 0, because the file was truncated
> 5. basebackup_read_file() throws a partial read error: "could not read
> file \"%s\": read %zu of %zu".
>
> If the truncation happened earlier, before the first read, or later,
> after the whole block has been read, you don't get the error.
>
> - Heikki
>
>
v19102026-0004-basebackup-bump-SINK_BUFFER_LENGTH-to-256k.patch
Description: Binary data
v19102026-0003-pg_basebackup-report-average-data-transfer.patch
Description: Binary data
v19102026-0002-pg_basebackup-add-new-client-blackhole-ben.patch
Description: Binary data
v19102026-0005-pg_basebackup-issue-posix_fadvise-for-more.patch
Description: Binary data
v19102026-0001-pg_basebackup-rename-the-blackhole-backup-.patch
Description: Binary data
