Fix unnecessary padding in incremental backups
authorTomas Vondra <tomas.vondra@postgresql.org>
Sun, 14 Apr 2024 18:34:29 +0000 (20:34 +0200)
committerTomas Vondra <tomas.vondra@postgresql.org>
Sun, 14 Apr 2024 18:37:49 +0000 (20:37 +0200)
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

index 3f1de3ed9ded4d8588603a6117fc2690fc177477..9a2bf59e84ee4a14ea5dfe24940aa146851084d9 100644 (file)
@@ -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));