| From: | David Steele <david(at)pgbackrest(dot)org> |
|---|---|
| To: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Avoid truncated or zero-padded copies of replaced files in backups. |
| Date: | 2026-10-24 21:34:56 |
| Message-ID: | 1f8fa143-9079-49e4-a599-a775d40e299f@pgbackrest.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| basebackup-replaced-file-v1.patch | text/plain | 4.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Greg Burd | 2026-10-24 21:48:37 | Re: Let an ordering index scan hand its ORDER BY value to the target list |
| Previous Message | Daniel Gustafsson | 2026-10-24 20:52:57 | Re: [PATCH] Keep git-external-diff usage text from executing |