database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-10-29 15:58:33 +03:00
Code Activity

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

Browse Source 
Base backups call stat() on each file while scanning the data directory,
and later send exactly that many bytes.  If the file is replaced by for
example a concurrent rename(), the backup contains the new file
truncated or zero-padded to the size of the old one.  This is harmless
for relation files as WAL replay repairs them.  However, it causes harm
for non-relation files manipulated as temporary file renamed in place.

Here is a list of non-relation files that use a temporary file followed
by a final rename() (I may be missing some):
* postgresql.auto.conf
* pg_logical/replorigin_checkpoint (a damaged copy fails its CRC check
and prevents startup)
* pg_prewarm's autoprewarm.blocks
* pg_stash_advice.tsv.

When sending files that are not relation files, fstat() is called again
on the opened file and the refreshed stats are used for the tar header
and the backup manifest.  Since the rename
is atomic, the opened file is always complete.  Relation files keep using
the earlier stat, to avoid copying data that WAL replay would restore
anyway (because that's cheaper).

Back-patch down to v15.  v14 is going to be EOL soon, so I have decided
to leave it out of the picture (risk/reward thing).

Author: David Steele <david@pgbackrest.org>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Shihao Zhong <zhong950419@gmail.com>
Discussion: https://postgr.es/m/CAGRkXqS_On-pLgcv6xjH8mvGhdcPmfWtythW3r8pYKUG+hsiuQ@mail.gmail.com
Backpatch-through: 15
This commit is contained in:
Michael Paquier
2026-10-27 13:53:26 +09:00
parent 99ea4a5527
commit 51d7bf5edc
1 changed files with 24 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
src/backend/backup/basebackup.c
View File

@@ -93,7 +93,7 @@ static int64 sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeo
backup_manifest_info *manifest, Oid spcoid,
IncrementalBackupInfo *ib);
static bool sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
struct stat *statbuf, bool missing_ok,
const struct stat *statbuf, bool missing_ok,
Oid dboid, Oid spcoid, RelFileNumber relfilenumber,
unsigned segno,
backup_manifest_info *manifest,
@@ -1575,12 +1575,15 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
* If the whole file is to be sent, then incremental_blocks should be NULL,
* and num_incremental_blocks can have any value, as it will be ignored.
*
* For non-relation files, the stat data of the opened file is used, not
* 'statbuf'.
*
* Returns true if the file was successfully sent, false if 'missing_ok',
* and the file did not exist.
*/
static bool
sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
struct stat *statbuf, bool missing_ok, Oid dboid, Oid spcoid,
const struct stat *statbuf, bool missing_ok, Oid dboid, Oid spcoid,
RelFileNumber relfilenumber, unsigned segno,
backup_manifest_info *manifest, unsigned num_incremental_blocks,
BlockNumber *incremental_blocks, unsigned truncation_block_length)
@@ -1593,6 +1596,7 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
bool verify_checksum = false;
pg_checksum_context checksum_ctx;
int ibindex = 0;
struct stat filestat;
if (pg_checksum_init(&checksum_ctx, manifest->checksum_type) < 0)
elog(ERROR, "could not initialize checksum of file \"%s\"",
@@ -1608,7 +1612,18 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
errmsg("could not open file \"%s\": %m", readfilename)));
}
_tarWriteHeader(sink, tarfilename, NULL, statbuf, false);
/*
* Non-relation files may be replaced by a rename(), making irrelevant the
* caller's stat data. Reuse fstat() on the opened file.
*/
filestat = *statbuf;
if (!RelFileNumberIsValid(relfilenumber) && fstat(fd, &filestat) != 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not stat file \"%s\": %m", readfilename)));
_tarWriteHeader(sink, tarfilename, NULL, &filestat, false);
/*
* Checksums are verified in multiples of BLCKSZ, so the buffer length
@@ -1697,13 +1712,13 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
*/
if (incremental_blocks == NULL)
{
size_t remaining = statbuf->st_size - bytes_done;
size_t remaining = filestat.st_size - bytes_done;
/*
* If we've read the required number of bytes, then it's time to
* stop.
*/
if (bytes_done >= statbuf->st_size)
if (bytes_done >= filestat.st_size)
break;
/*
@@ -1798,9 +1813,9 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
}
/* If the file was truncated while we were sending it, pad it with zeros */
while (bytes_done < statbuf->st_size)
while (bytes_done < filestat.st_size)
{
size_t remaining = statbuf->st_size - bytes_done;
size_t remaining = filestat.st_size - bytes_done;
size_t nbytes = Min(sink->bbs_buffer_length, remaining);
MemSet(sink->bbs_buffer, 0, nbytes);
@@ -1835,8 +1850,8 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
total_checksum_failures += checksum_failures;
AddFileToBackupManifest(manifest, spcoid, tarfilename, statbuf->st_size,
(pg_time_t) statbuf->st_mtime, &checksum_ctx);
AddFileToBackupManifest(manifest, spcoid, tarfilename, filestat.st_size,
(pg_time_t) filestat.st_mtime, &checksum_ctx);
return true;
}