Align blocks in incremental backups to BLCKSZ

Align blocks stored in incremental files to BLCKSZ, so that the
incremental backups work well with CoW filesystems.

The header of the incremental file is padded with \0 to a multiple of
BLCKSZ, so that the block data (also BLCKSZ) is aligned to BLCKSZ. The
padding is added only to files containing block data, so files with just
the header remain small. This adds a bit of extra space, but as the
number of blocks increases the overhead gets negligible very quickly.
And as the padding is \0 bytes, it does compress extremely well.

The alignment is important for CoW filesystems that usually require the
blocks to be aligned to filesystem page size for features like block
sharing, deduplication etc. to work well. With the variable sized header
the blocks in the increments were not aligned at all, negating the
benefits of the CoW filesystems.

This matters even for non-CoW filesystems, for example when placed on a
RAID array. If the block is not aligned, it may easily span multiple
devices, causing read and write amplification.

It might be better to align the blocks to the filesystem page, not
BLCKSZ, but we have no good way to determine that. Even if we determine
the page size at the time of taking the backup, the backup may move. For
now the BLCKSZ seems sufficient - the filesystem page is usually 4K, so
the default BLCKSZ (8K by default) is aligned to that.

Author: Tomas Vondra
Reviewed-by: Robert Haas, Jakub Wartak
Discussion: https://postgr.es/m/3024283a-7491-4240-80d0-421575f6bb23%40enterprisedb.com

src/backend/backup/basebackup.c

@@ -1623,6 +1623,8 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
 	{
 		unsigned	magic = INCREMENTAL_MAGIC;
 		size_t		header_bytes_done = 0;
+		char		padding[BLCKSZ];
+		size_t		paddinglen;
 
 		/* Emit header data. */
 		push_to_sink(sink, &checksum_ctx, &header_bytes_done,
@@ -1635,6 +1637,23 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
 					 incremental_blocks,
 					 sizeof(BlockNumber) * num_incremental_blocks);
 
+		/*
+		 * 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.
+		 */
+		if (num_incremental_blocks > 0)
+		{
+			paddinglen = (BLCKSZ - (header_bytes_done % BLCKSZ));
+
+			memset(padding, 0, paddinglen);
+			bytes_done += paddinglen;
+
+			push_to_sink(sink, &checksum_ctx, &header_bytes_done,
+						 padding, paddinglen);
+		}
+
 		/* Flush out any data still in the buffer so it's again empty. */
 		if (header_bytes_done > 0)
 		{
@@ -1748,6 +1767,13 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename,
 		blkno += cnt / BLCKSZ;
 		bytes_done += cnt;
 
+		/*
+		 * Make sure incremental files with block data are properly aligned
+		 * (header is a multiple of BLCKSZ, blocks are BLCKSZ too).
+		 */
+		Assert(!((incremental_blocks != NULL && num_incremental_blocks > 0) &&
+				 (bytes_done % BLCKSZ != 0)));
+
 		/* Archive the data we just read. */
 		bbsink_archive_contents(sink, cnt);
 

src/backend/backup/basebackup_incremental.c

@@ -928,6 +928,36 @@ GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path,
 	return BACK_UP_FILE_INCREMENTALLY;
 }
 
+/*
+ * Compute the size for a header of an incremental file containing a given
+ * number of blocks. The header is rounded to a multiple of BLCKSZ, but
+ * only if the file will store some block data.
+ */
+extern size_t
+GetIncrementalHeaderSize(unsigned num_blocks_required)
+{
+	size_t		result;
+
+	/* Make sure we're not going to overflow. */
+	Assert(num_blocks_required <= RELSEG_SIZE);
+
+	/*
+	 * Three four byte quantities (magic number, truncation block length,
+	 * block count) followed by block numbers.
+	 */
+	result = 3 * sizeof(uint32) + (sizeof(BlockNumber) * num_blocks_required);
+
+	/*
+	 * Round the header size to a multiple of BLCKSZ - when not a multiple of
+	 * BLCKSZ, add the missing fraction of a block. But do this only if the
+	 * file will store data for some blocks, otherwise keep it small.
+	 */
+	if ((num_blocks_required > 0) && (result % BLCKSZ != 0))
+		result += BLCKSZ - (result % BLCKSZ);
+
+	return result;
+}
+
 /*
  * Compute the size for an incremental file containing a given number of blocks.
  */
@@ -940,11 +970,12 @@ GetIncrementalFileSize(unsigned num_blocks_required)
 	Assert(num_blocks_required <= RELSEG_SIZE);
 
 	/*
-	 * Three four byte quantities (magic number, truncation block length,
-	 * block count) followed by block numbers followed by block contents.
+	 * Header with three four byte quantities (magic number, truncation block
+	 * length, block count) followed by block numbers, rounded to a multiple
+	 * of BLCKSZ (for files with block data), followed by block contents.
 	 */
-	result = 3 * sizeof(uint32);
-	result += (BLCKSZ + sizeof(BlockNumber)) * num_blocks_required;
+	result = GetIncrementalHeaderSize(num_blocks_required);
+	result += BLCKSZ * num_blocks_required;
 
 	return result;
 }

src/bin/pg_combinebackup/reconstruct.c

@@ -472,6 +472,14 @@ make_incremental_rfile(char *filename)
 		sizeof(rf->truncation_block_length) +
 		sizeof(BlockNumber) * rf->num_blocks;
 
+	/*
+	 * Round header length to a multiple of BLCKSZ, so that blocks contents
+	 * are properly aligned. Only do this when the file actually has data for
+	 * some blocks.
+	 */
+	if ((rf->num_blocks > 0) && ((rf->header_length % BLCKSZ) != 0))
+		rf->header_length += (BLCKSZ - (rf->header_length % BLCKSZ));
+
 	return rf;
 }
 

src/include/backup/basebackup_incremental.h

@@ -51,5 +51,6 @@ extern FileBackupMethod GetFileBackupMethod(IncrementalBackupInfo *ib,
 											BlockNumber *relative_block_numbers,
 											unsigned *truncation_block_length);
 extern size_t GetIncrementalFileSize(unsigned num_blocks_required);
+extern size_t GetIncrementalHeaderSize(unsigned num_blocks_required);
 
 #endif