v8-0006-Refactor-split-verify_backup_file-function.patch

From 4be84391a1e05b70ca6fcb51e050a1a08de2b94d Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Thu, 1 Aug 2024 12:15:26 +0530
Subject: [PATCH v8 06/12] Refactor: split verify_backup_file() function.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Move the manifest entry verification code into a new function called
verify_manifest_entry(). Also, move the total size computation code
into another new function called compute_total_size(), which will be
called from the main function.

The current computation is designed for plain backups, which operate
in two rounds. In the first round, only the files are checked against
the manifest backup, and their sizes are added to the total size for
progress reporting. In the second round, the actual files are read for
the checksums verification, and the read size progress is noted and
reported.

However, for tar backups, we do not operate in two rounds because TAR
format files do not allow random access like plain backups. Therefore,
we verify the file entries against the backup manifest and right
after, perform the checksum verification in a single pass. That’s why
we need to compute the total size before starting the TAR backup
verification pass.
---
 src/bin/pg_verifybackup/pg_verifybackup.c | 82 ++++++++++++++++++-----
 src/bin/pg_verifybackup/pg_verifybackup.h |  3 +
 2 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index 4e42757c346..8eadaac72e3 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -72,6 +72,7 @@ static void parse_required_wal(verifier_context *context,
 							   char *pg_waldump_path,
 							   char *wal_directory);
 
+static void compute_total_size(verifier_context *context);
 static void usage(void);
 
 static const char *progname;
@@ -250,6 +251,13 @@ main(int argc, char **argv)
 	 */
 	context.manifest = parse_manifest_file(manifest_path);
 
+	/*
+	 * For the progress report, compute the total size of the files to be
+	 * read, which occurs only when checksum verification is enabled.
+	 */
+	if (!context.skip_checksums)
+		compute_total_size(&context);
+
 	/*
 	 * Now scan the files in the backup directory. At this stage, we verify
 	 * that every file on disk is present in the manifest and that the sizes
@@ -614,6 +622,27 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath)
 		return;
 	}
 
+	/* Check the backup manifest entry for this file. */
+	m = verify_manifest_entry(context, relpath, sb.st_size);
+
+	/*
+	 * Validate the manifest system identifier, not available in manifest
+	 * version 1.
+	 */
+	if (context->manifest->version != 1 &&
+		strcmp(relpath, "global/pg_control") == 0 &&
+		m != NULL && m->matched && !m->bad)
+		verify_control_file(fullpath, context->manifest->system_identifier);
+}
+
+/*
+ * Verify file and its size entry in the manifest.
+ */
+manifest_file *
+verify_manifest_entry(verifier_context *context, char *relpath, int64 filesize)
+{
+	manifest_file *m;
+
 	/* Check whether there's an entry in the manifest hash. */
 	m = manifest_files_lookup(context->manifest->files, relpath);
 	if (m == NULL)
@@ -621,40 +650,29 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath)
 		report_backup_error(context,
 							"\"%s\" is present on disk but not in the manifest",
 							relpath);
-		return;
+		return NULL;
 	}
 
 	/* Flag this entry as having been encountered in the filesystem. */
 	m->matched = true;
 
 	/* Check that the size matches. */
-	if (m->size != sb.st_size)
+	if (m->size != filesize)
 	{
 		report_backup_error(context,
 							"\"%s\" has size %lld on disk but size %zu in the manifest",
-							relpath, (long long int) sb.st_size, m->size);
+							relpath, (long long int) filesize, m->size);
 		m->bad = true;
 	}
 
-	/*
-	 * Validate the manifest system identifier, not available in manifest
-	 * version 1.
-	 */
-	if (context->manifest->version != 1 &&
-		strcmp(relpath, "global/pg_control") == 0)
-		verify_control_file(fullpath, context->manifest->system_identifier);
-
-	/* Update statistics for progress report, if necessary */
-	if (context->show_progress && !context->skip_checksums &&
-		should_verify_checksum(m))
-		context->total_size += m->size;
-
 	/*
 	 * We don't verify checksums at this stage. We first finish verifying that
 	 * we have the expected set of files with the expected sizes, and only
 	 * afterwards verify the checksums. That's because computing checksums may
 	 * take a while, and we'd like to report more obvious problems quickly.
 	 */
+
+	return m;
 }
 
 /*
@@ -817,7 +835,7 @@ verify_file_checksum(verifier_context *context, manifest_file *m,
 
 	/*
 	 * Double-check that we read the expected number of bytes from the file.
-	 * Normally, a file size mismatch would be caught in verify_backup_file
+	 * Normally, a file size mismatch would be caught in verify_manifest_entry
 	 * and this check would never be reached, but this provides additional
 	 * safety and clarity in the event of concurrent modifications or
 	 * filesystem misbehavior.
@@ -988,6 +1006,36 @@ progress_report(verifier_context *context, bool finished)
 	fputc((!finished && isatty(fileno(stderr))) ? '\r' : '\n', stderr);
 }
 
+/*
+ * Compute the total size of backup files for progress reporting.
+ */
+static void
+compute_total_size(verifier_context *context)
+{
+	manifest_data *manifest = context->manifest;
+	manifest_files_iterator it;
+	manifest_file *m;
+	uint64		total_size = 0;
+
+	if (!context->show_progress)
+		return;
+
+	manifest_files_start_iterate(manifest->files, &it);
+	while ((m = manifest_files_iterate(manifest->files, &it)) != NULL)
+	{
+		/*
+		 * We are not going to read files that are ignored or whose checksums
+		 * are not calculated, so their sizes should be excluded from the
+		 * total.
+		 */
+		if (!should_ignore_relpath(context, m->pathname) &&
+			m->checksum_type != CHECKSUM_TYPE_NONE)
+			total_size += m->size;
+	}
+
+	context->total_size = total_size;
+}
+
 /*
  * Print out usage information and exit.
  */
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.h b/src/bin/pg_verifybackup/pg_verifybackup.h
index 90900048547..98c75916255 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.h
+++ b/src/bin/pg_verifybackup/pg_verifybackup.h
@@ -105,6 +105,9 @@ typedef struct verifier_context
 	uint64		done_size;
 } verifier_context;
 
+extern manifest_file *verify_manifest_entry(verifier_context *context,
+											char *relpath, int64 filesize);
+
 extern void report_backup_error(verifier_context *context,
 								const char *pg_restrict fmt,...)
 			pg_attribute_printf(2, 3);
-- 
2.18.0