v20240401-0005-use-copy_file_range-with-checksums.patch

From 6f21b1a635ec0f65a525238698d94953635ab52b Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 30 Mar 2024 17:41:27 +0100
Subject: [PATCH v20240401 5/7] use copy_file_range with checksums

Use copy_file_range even when having to calculate checksum. Does
extra pg_pread() to read the data, but the data is still copies
using copy_file_range().
---
 src/bin/pg_combinebackup/pg_combinebackup.c | 29 +++++++++
 src/bin/pg_combinebackup/reconstruct.c      | 70 +++++++++++++--------
 2 files changed, 74 insertions(+), 25 deletions(-)

diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 76a63d826a7..4f3d7747ce6 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -223,6 +223,35 @@ main(int argc, char *argv[])
 	if (opt.no_manifest)
 		opt.manifest_checksums = CHECKSUM_TYPE_NONE;
 
+	/* Check that the platform supports the requested copy method. */
+	if (opt.copy_method == COPY_METHOD_CLONE)
+	{
+#if (defined(HAVE_COPYFILE) && defined(COPYFILE_CLONE_FORCE)) || \
+	(defined(__linux__) && defined(FICLONE))
+
+		if (opt.dry_run)
+			pg_log_debug("would use cloning to copy files");
+		else
+			pg_log_debug("will use cloning to copy files");
+
+#else
+		pg_fatal("file cloning not supported on this platform");
+#endif
+	}
+	else if (opt.copy_method == COPY_METHOD_COPY_FILE_RANGE)
+	{
+#if defined(HAVE_COPY_FILE_RANGE)
+
+		if (opt.dry_run)
+			pg_log_debug("would use copy_file_range to copy blocks");
+		else
+			pg_log_debug("will use copy_file_range to copy blocks");
+
+#else
+		pg_fatal("copy_file_range not supported on this platform");
+#endif
+	}
+
 	/* Read the server version from the final backup. */
 	version = read_pg_version_file(argv[argc - 1]);
 
diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c
index f02c91a5d4e..e4081ddfcec 100644
--- a/src/bin/pg_combinebackup/reconstruct.c
+++ b/src/bin/pg_combinebackup/reconstruct.c
@@ -629,6 +629,9 @@ write_reconstructed_file(char *input_filename,
 		rfile	   *s = sourcemap[i];
 		int			wb;
 
+		bool		skip_page_read = false;
+		bool		use_copy_range = false;
+
 		/* Update accounting information. */
 		if (s == NULL)
 			++zero_blocks;
@@ -644,28 +647,23 @@ write_reconstructed_file(char *input_filename,
 			continue;
 
 		/*
-		 * If requested, copy the block using copy_file_range.
+		 * Do we need to actually read the souce block?
 		 *
-		 * We can'd do this if the block needs to be zero-filled or when we
-		 * need to update checksum.
+		 * We only need to read the block if we are to perform plain copy or
+		 * calculate the checksum. If the user requested copy_file_range, we
+		 * may be able to skip reading the block.
 		 */
-		if ((copy_method == COPY_METHOD_COPY_FILE_RANGE) &&
-			(s != NULL) && (checksum_ctx->type == CHECKSUM_TYPE_NONE))
-		{
-#if defined(HAVE_COPY_FILE_RANGE)
-			wb = copy_file_range(s->fd, &offsetmap[i], wfd, NULL, BLCKSZ, 0);
+		skip_page_read = ((copy_method == COPY_METHOD_COPY_FILE_RANGE) &&
+						  (checksum_ctx->type == CHECKSUM_TYPE_NONE));
 
-			if (wb < 0)
-				pg_fatal("error while copying file range from \"%s\" to \"%s\": %m",
-						 input_filename, output_filename);
-			else if (wb != BLCKSZ)
-				pg_fatal("could not write file \"%s\": wrote only %d of %d bytes",
-						 output_filename, wb, BLCKSZ);
-#else
-			pg_fatal("copy_file_range not supported on this platform");
-#endif
-			continue;
-		}
+		/*
+		 * Should we copy the block using copy_file_range?
+		 *
+		 * We can do that if the user requested that, and the block is not
+		 * going to be zero-filled.
+		 */
+		use_copy_range = ((copy_method == COPY_METHOD_COPY_FILE_RANGE) &&
+						  (s != NULL));
 
 		/* Read or zero-fill the block as appropriate. */
 		if (s == NULL)
@@ -676,7 +674,7 @@ write_reconstructed_file(char *input_filename,
 			 */
 			memset(buffer, 0, BLCKSZ);
 		}
-		else
+		else if (!skip_page_read)
 		{
 			int			rb;
 
@@ -707,17 +705,39 @@ write_reconstructed_file(char *input_filename,
 			}
 		}
 
-		/* Write out the block. */
-		if ((wb = write(wfd, buffer, BLCKSZ)) != BLCKSZ)
+		/*
+		 * If possible, copy the block using copy_file_range. If not possible
+		 * (not requested/supported, or the block is zero-filled), fallback to
+		 * the regular write.
+		 */
+		if (use_copy_range)
 		{
+			wb = copy_file_range(s->fd, &offsetmap[i], wfd, NULL, BLCKSZ, 0);
+
 			if (wb < 0)
-				pg_fatal("could not write file \"%s\": %m", output_filename);
-			else
+				pg_fatal("error while copying file range from \"%s\" to \"%s\": %m",
+						 input_filename, output_filename);
+			else if (wb != BLCKSZ)
 				pg_fatal("could not write file \"%s\": wrote only %d of %d bytes",
 						 output_filename, wb, BLCKSZ);
 		}
+		else
+		{
+			/* Write out the block. */
+			if ((wb = write(wfd, buffer, BLCKSZ)) != BLCKSZ)
+			{
+				if (wb < 0)
+					pg_fatal("could not write file \"%s\": %m", output_filename);
+				else
+					pg_fatal("could not write file \"%s\": wrote only %d of %d bytes",
+							 output_filename, wb, BLCKSZ);
+			}
+		}
 
-		/* Update the checksum computation. */
+		/*
+		 * Update the checksum computation (we must have read the page if we're
+		 * to calculate the checksum).
+		 */
 		if (pg_checksum_update(checksum_ctx, buffer, BLCKSZ) < 0)
 			pg_fatal("could not update checksum of file \"%s\"",
 					 output_filename);
-- 
2.44.0