Avoid truncated or zero-padded copies of replaced files in backups.

Started by David Steeleabout 6 hours ago1 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t254374
psql -h localhost -U postgres

Built from patchset v1 (message #1), October 24, 2026 at 09:45 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t254374_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t254374_1 && git checkout t254374_1

Patchset v1 (message #1) is on t254374_1

Jump to latest
#1David Steele
david@pgbackrest.org

Hackers,

Base backups stat each file while scanning the data directory and later
send exactly the number of bytes reported by the stat. If the file is
shorter when it is read then the copy is zero-padded. This is harmless
for relation files since any data written after the stat will be
restored from WAL.

Some files that are not relation files are written to a temp file and
then renamed into place, e.g. postgresql.auto.conf by ALTER SYSTEM and
pg_logical/replorigin_checkpoint by every checkpoint. If the rename
happens between the stat and the open then the backup contains the new
file truncated or zero-padded to the size of the old file. WAL replay
does not repair these files. A damaged replorigin_checkpoint fails its
CRC check and the restored cluster will not start. A damaged
postgresql.auto.conf either fails to parse or, if the truncation falls
on a line boundary, silently loses settings.

Some contrib modules also write files to the data directory this way.
pg_prewarm rewrites autoprewarm.blocks every
pg_prewarm.autoprewarm_interval. A truncated autoprewarm.blocks fails to
load. The autoprewarm worker then exits and is not restarted so blocks
are neither prewarmed nor dumped. pg_stash_advice rewrites
pg_stash_advice.tsv every pg_stash_advice.persist_interval when the
stashed advice has changed. A truncated pg_stash_advice.tsv may load a
truncated advice string without error since the last line is not
required to end with a newline.

For files that are not relation files, fstat() the opened file and use
that stat for the tar header and the backup manifest. Since the rename
is atomic the opened file is always complete. Relation files are not
replaced this way and are still sent using the earlier stat. Calling
fstat() for every file would be simpler, but using the earlier stat for
relation files means that data added to relation files after the stat is
not copied. WAL replay will restore that data anyway so copying it is
wasteful.

This should be back-patched to all supported branches. Before v17,
sendFile() does not receive the relfilenumber so on those branches
sendDir() should save the result of its existing
parse_filename_for_nontemp_relation() call and pass it to sendFile().
That call is only made for database directories so relation files in
global/ are not identified and get the stat of the opened file. The copy
may then include data written between the earlier stat and the open,
which is harmless since WAL replay will overwrite that data, just as it
overwrites blocks modified while the file is being copied.

Claude Code was used to audit the server and contrib modules for files
that are written to a temp file and then renamed into place, but the
actual issue was discovered independently by me. I had just never found
the time to do the audit to determine if it was real or imagined, though
I suspected postgresql.auto.conf at least would be an issue.

Regards,
-David

Attachments:

t254374_1
basebackup-replaced-file-v1.patchtext/plain; charset=UTF-8; name=basebackup-replaced-file-v1.patchDownload+18-1