| From: | Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Cc: | Gustavo William <gustavowilliam0805(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: enhancing pg_basebackup speeds up to ~23Gbps (small fixes + io_uring/Direct I/O) |
| Date: | 2026-10-07 09:55:33 |
| Message-ID: | CAKZiRmyLiT2j7jyFmtUFOTtV0BSULN9X22pra9AiBLEH1EVyzQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Oct 6, 2026 at 5:42 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> 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.
0001..0004 without asserts, still give ~1.67x on hot DB fully in VFS cache:
pg_basebackup -c fast -v -h 127.0.0.1 --target=client-blackhole -Xnone \
-c fast --no-sync --no-verify-checksums --manifest-checksums=NONE \
2>&1 | grep MB/s
(client-blackhole still transfers to CLI, so it's receieving just ignoring
write path at all)
256kb (as within 0004):
pg_basebackup: base backup completed (avg 5503.2 MB/s)
pg_basebackup: base backup completed (avg 5331.5 MB/s)
pg_basebackup: base backup completed (avg 5205.6 MB/s)
32kB:
pg_basebackup: base backup completed (avg 3110.2 MB/s)
pg_basebackup: base backup completed (avg 3307.3 MB/s)
pg_basebackup: base backup completed (avg 3191.2 MB/s)
I've tried a couple of other integrity tests too for this new retry code:
1. high concurrency pgbench with loop of parallel pg_basebackup +
pg_verifybackup for couple of minutes (to see if that backing up
files being appended works OK). It went OK.
2. I've tried failure injection on 3rd invocation of pread of certain file:
strace -ff -p $POSTMASTERPID -P $PGDATA/base/1/1255 \
-e trace=pread64 -k -e inject=pread64:error=EINTR:when=3
and in first version of my patch I've got:
WARNING: aborting backup due to backend exiting before pg_backup_stop ..
ERROR: could not read file "./base/1/1255": Interrupted system call
strace got it exactly there:
pread64(14, ..., 262144, 524288) = -1 EINTR (Interrupted system
call) (INJECTED)
> /usr/lib/x86_64-linux-gnu/libc.so.6(pread64+0x17) [0xfa577]
> .../bin/postgres(basebackup_read_file+0x61) [0x23b9a1]
> .../bin/postgres(read_file_data_into_buffer+0x4d) [0x23bcfd]
so right, today even master seems to be vulnerable to this I think.
Anyway, I've added to 0004 the EINTR guard (FileReadV() had goto for this)
and I have retested this using the EINTR injection for way more pgdata
files at specifc blocks and then checked if the resulting DB was fine:
rm -rf /tmp/full1
# ~10GB DB
pg_basebackup -c fast -v -h 127.0.0.1 -D /tmp/full1 -c fast
# strace on some relations with "when=1..2+1" has reported INJECTED
# messages (causing restart of the syscall in code)
pg_verifybackup /tmp/full1
and got "backup successfully verified"
3. With SIMULATE_SHORT_READ it also works OK.
-J.
| Attachment | Content-Type | Size |
|---|---|---|
| v07102026-0004-basebackup-bump-SINK_BUFFER_LENGTH-to-256k.patch | application/x-patch | 5.9 KB |
| v07102026-0003-pg_basebackup-report-average-data-transfer.patch | application/x-patch | 3.1 KB |
| v07102026-0001-pg_basebackup-rename-the-blackhole-backup-.patch | application/x-patch | 5.3 KB |
| v07102026-0002-pg_basebackup-add-new-client-blackhole-ben.patch | application/x-patch | 16.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | David Geier | 2026-10-07 09:55:28 | Re: Hash a ScalarArrayOpExpr whose array is fixed for one execution |