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

> On Oct 25, 2026, at 05:34, David Steele <david(at)pgbackrest(dot)org> wrote:
>
> 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<basebackup-replaced-file-v1.patch>

Hi David,

Thanks for the patch. Overall LGTM. A couple of really nit comments:

1
```
+ * If the file is not a relation file then statbuf is replaced with the stat of
+ * the file that was opened.
```

TBH, this comment is not easy to read for non-native English speakers like me. I feel that the code change itself is easier to understand than this comment. So, how about making it more explicit, for example:
```
* For non-relation files, statbuf is updated using fstat() on the opened file.
```

2
```
+ /*
+ * If this is not a relation file then use the stat of the file that was
+ * actually opened rather than the caller's earlier stat. Some files, e.g.
+ * postgresql.auto.conf, are written to a temp file and then renamed in
+ * place. If the rename happened after the earlier stat then the opened
+ * file may have a different size and sending the earlier size would
+ * truncate or zero-pad the file. WAL replay will not repair these files.
+ * Relation files are not replaced this way and any data added to relation
+ * files after the earlier stat will be restored by WAL replay.
+ */
```

I think we can leave the detailed explanation in the commit message. Here, explaining the race and why we need the opened file's stat should be enough. So, how about shortening it to:
```
/*
* Non-relation files may be replaced by rename between the caller's stat
* and our open. Use the opened file's stat to avoid truncating or
* zero-padding its contents, which WAL replay cannot repair.
*/
```

These are really nit comments, feel free if you don’t want to take them.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/