From cd4b6af620975bb9802c464a06d2289d2f97cbf4 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sun, 14 Apr 2024 20:34:29 +0200 Subject: [PATCH] Fix unnecessary padding in incremental backups Commit 10e3226ba13d added padding to incremental backups to ensure the block data is properly aligned. The code in sendFile() however failed to consider that the header may be a multiple of BLCKSZ and thus already aligned, adding a full BLCKSZ of unnecessary padding. Not only does this make the incremental file a bit larger, but the other places calculating the amount of padding did realize it's not needed and did not include it in the formula. This resulted in pg_basebackup getting confused while parsing the data stream, trying to access files with invalid filenames (e.g. with binary data etc.) and failing. --- src/backend/backup/basebackup.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index 3f1de3ed9d..9a2bf59e84 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -1638,11 +1638,12 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, /* * Add padding to align header to a multiple of BLCKSZ, but only if - * the incremental file has some blocks. If there are no blocks we - * don't want make the file unnecessarily large, as that might make - * some filesystem optimizations impossible. + * the incremental file has some blocks, and the alignment is actually + * needed (i.e. header is not already a multiple of BLCKSZ). If there + * are no blocks we don't want to make the file unnecessarily large, + * as that might make some filesystem optimizations impossible. */ - if (num_incremental_blocks > 0) + if ((num_incremental_blocks > 0) && (header_bytes_done % BLCKSZ != 0)) { paddinglen = (BLCKSZ - (header_bytes_done % BLCKSZ));