Fix unnecessary padding in incremental backups

tvondra · tvondra · commit cd4b6af62097 · 2024-04-14T20:37:49.000+02:00
Commit 10e3226 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.
diff --git 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));
 
