0004-Make-checksum-optional-in-pg_basebackup-review-comme.patch

From df11758f8310a3bd3b3622d58651e8b09c7c6569 Mon Sep 17 00:00:00 2001
From: Rushabh Lathia <rushabh.lathia@enterprisedb.com>
Date: Fri, 22 Nov 2019 15:17:47 +0530
Subject: [PATCH 4/5] Make checksum optional in pg_basebackup & review
 comments.

---
 src/backend/replication/basebackup.c   | 73 +++++++++++++++++++++++-----------
 src/backend/replication/repl_gram.y    |  6 +++
 src/backend/replication/repl_scanner.l |  1 +
 src/bin/pg_basebackup/pg_basebackup.c  | 12 +++++-
 4 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 273f837..66aa0fc 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -132,6 +132,9 @@ static long long int total_checksum_failures;
 /* Do not verify checksums. */
 static bool noverify_checksums = false;
 
+/* Add file entry in to manifest with checksums. */
+static bool manifest_with_checksums = false;
+
 /*
  * The contents of these directories are removed or recreated during server
  * start so they are not included in backups.  The directories themselves are
@@ -762,6 +765,15 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 			noverify_checksums = true;
 			o_noverify_checksums = true;
 		}
+		else if (strcmp(defel->defname, "manifest_with_checksums") == 0)
+		{
+			if (manifest_with_checksums)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("duplicate option \"%s\"", defel->defname)));
+			manifest_with_checksums = true;
+		}
+
 		else
 			elog(ERROR, "option \"%s\" not recognized",
 				 defel->defname);
@@ -930,15 +942,18 @@ AddFileToManifest(StringInfo manifest, const char *tsoid,
 	pg_strftime(timebuf, sizeof(timebuf), "%Y-%m-%d %H:%M:%S %Z", tm);
 
 	/* Convert checksum to hexadecimal. */
-	shatextlen =
-		hex_encode((char *) shabuf, PG_SHA256_DIGEST_LENGTH, shatextbuf);
-	Assert(shatextlen + 1 == sizeof(shatextbuf));
-	shatextbuf[shatextlen] = '\0';
+	if (manifest_with_checksums)
+	{
+		shatextlen =
+			hex_encode((char *) shabuf, PG_SHA256_DIGEST_LENGTH, shatextbuf);
+		Assert(shatextlen + 1 == sizeof(shatextbuf));
+		shatextbuf[shatextlen] = '\0';
+	}
 
 	/* Add to manifest. */
 	appendStringInfo(manifest, "File\t%s\t%zu\t%s\t%s\n",
 					 escaped_filename == NULL ? filename : escaped_filename,
-					 size, timebuf, shatextbuf);
+					 size, timebuf, manifest_with_checksums ? shatextbuf : "-");
 
 	/* Avoid leaking memory. */
 	if (escaped_filename != NULL)
@@ -957,17 +972,20 @@ SendBackupManifest(StringInfo manifest)
 	int				shastringlen;
 
 	/* Checksum the manifest. */
-	pg_sha256_init(&sha256_ctx);
-	pg_sha256_update(&sha256_ctx, (uint8 *) manifest->data, manifest->len);
-	pg_sha256_final(&sha256_ctx, shabuf);
-	appendStringInfoString(manifest, "Manifest-Checksum\t");
-	shastringlen = PG_SHA256_DIGEST_LENGTH * 2;
-	enlargeStringInfo(manifest, shastringlen);
-	shastringlen = hex_encode((char *) shabuf, PG_SHA256_DIGEST_LENGTH,
-							  manifest->data + manifest->len);
-	Assert(shastringlen == PG_SHA256_DIGEST_LENGTH * 2);
-	manifest->len += shastringlen;
-	appendStringInfoChar(manifest, '\n');
+	if (manifest_with_checksums)
+	{
+		pg_sha256_init(&sha256_ctx);
+		pg_sha256_update(&sha256_ctx, (uint8 *) manifest->data, manifest->len);
+		pg_sha256_final(&sha256_ctx, shabuf);
+		appendStringInfoString(manifest, "Manifest-Checksum\t");
+		shastringlen = PG_SHA256_DIGEST_LENGTH * 2;
+		enlargeStringInfo(manifest, shastringlen);
+		shastringlen = hex_encode((char *) shabuf, PG_SHA256_DIGEST_LENGTH,
+				manifest->data + manifest->len);
+		Assert(shastringlen == PG_SHA256_DIGEST_LENGTH * 2);
+		manifest->len += shastringlen;
+		appendStringInfoChar(manifest, '\n');
+	}
 
 	/* Send CopyOutResponse message */
 	pq_beginmessage(&protobuf, 'H');
@@ -1100,7 +1118,8 @@ sendFileWithContent(const char *filename, const char *content,
 	pg_sha256_ctx	sha256_ctx;
 	uint8		shabuf[PG_SHA256_DIGEST_LENGTH];
 
-	pg_sha256_init(&sha256_ctx);
+	if (manifest_with_checksums)
+		pg_sha256_init(&sha256_ctx);
 
 	len = strlen(content);
 
@@ -1134,8 +1153,12 @@ sendFileWithContent(const char *filename, const char *content,
 		pq_putmessage('d', buf, pad);
 	}
 
-	pg_sha256_update(&sha256_ctx, (uint8 *) content, len);
-	pg_sha256_final(&sha256_ctx, shabuf);
+	if (manifest_with_checksums)
+	{
+		pg_sha256_update(&sha256_ctx, (uint8 *) content, len);
+		pg_sha256_final(&sha256_ctx, shabuf);
+	}
+
 	AddFileToManifest(manifest, NULL, filename, len, statbuf.st_mtime,
 					  shabuf);
 }
@@ -1571,7 +1594,8 @@ sendFile(const char *readfilename, const char *tarfilename,
 	pg_sha256_ctx	sha256_ctx;
 	uint8		shabuf[PG_SHA256_DIGEST_LENGTH];
 
-	pg_sha256_init(&sha256_ctx);
+	if (manifest_with_checksums)
+		pg_sha256_init(&sha256_ctx);
 
 	fp = AllocateFile(readfilename, "rb");
 	if (fp == NULL)
@@ -1742,7 +1766,8 @@ sendFile(const char *readfilename, const char *tarfilename,
 					(errmsg("base backup could not send data, aborting backup")));
 
 		/* Also feed it to the checksum machinery. */
-		pg_sha256_update(&sha256_ctx, (uint8 *) buf, cnt);
+		if (manifest_with_checksums)
+			pg_sha256_update(&sha256_ctx, (uint8 *) buf, cnt);
 
 		len += cnt;
 		throttle(cnt);
@@ -1768,7 +1793,8 @@ sendFile(const char *readfilename, const char *tarfilename,
 		{
 			cnt = Min(sizeof(buf), statbuf->st_size - len);
 			pq_putmessage('d', buf, cnt);
-			pg_sha256_update(&sha256_ctx, (uint8 *) buf, cnt);
+			if (manifest_with_checksums)
+				pg_sha256_update(&sha256_ctx, (uint8 *) buf, cnt);
 			len += cnt;
 			throttle(cnt);
 		}
@@ -1801,7 +1827,8 @@ sendFile(const char *readfilename, const char *tarfilename,
 
 	total_checksum_failures += checksum_failures;
 
-	pg_sha256_final(&sha256_ctx, shabuf);
+	if (manifest_with_checksums)
+		pg_sha256_final(&sha256_ctx, shabuf);
 	AddFileToManifest(manifest, tsoid, tarfilename, statbuf->st_size,
 					  statbuf->st_mtime, shabuf);
 
diff --git a/src/backend/replication/repl_gram.y b/src/backend/replication/repl_gram.y
index c4e11cc..542a3f7 100644
--- a/src/backend/replication/repl_gram.y
+++ b/src/backend/replication/repl_gram.y
@@ -87,6 +87,7 @@ static SQLCmd *make_sqlcmd(void);
 %token K_EXPORT_SNAPSHOT
 %token K_NOEXPORT_SNAPSHOT
 %token K_USE_SNAPSHOT
+%token K_MANIFEST_WITH_CHECKSUMS
 
 %type <node>	command
 %type <node>	base_backup start_replication start_logical_replication
@@ -214,6 +215,11 @@ base_backup_opt:
 				  $$ = makeDefElem("noverify_checksums",
 								   (Node *)makeInteger(true), -1);
 				}
+			| K_MANIFEST_WITH_CHECKSUMS
+				{
+				  $$ = makeDefElem("manifest_with_checksums",
+								   (Node *)makeInteger(true), -1);
+				}
 			;
 
 create_replication_slot:
diff --git a/src/backend/replication/repl_scanner.l b/src/backend/replication/repl_scanner.l
index 380faeb..4f92bc1 100644
--- a/src/backend/replication/repl_scanner.l
+++ b/src/backend/replication/repl_scanner.l
@@ -107,6 +107,7 @@ EXPORT_SNAPSHOT		{ return K_EXPORT_SNAPSHOT; }
 NOEXPORT_SNAPSHOT	{ return K_NOEXPORT_SNAPSHOT; }
 USE_SNAPSHOT		{ return K_USE_SNAPSHOT; }
 WAIT				{ return K_WAIT; }
+MANIFEST_WITH_CHECKSUMS { return K_MANIFEST_WITH_CHECKSUMS; }
 
 ","				{ return ','; }
 ";"				{ return ';'; }
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 508e4a6..f4f8ffe 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -141,6 +141,7 @@ static bool temp_replication_slot = true;
 static bool create_slot = false;
 static bool no_slot = false;
 static bool verify_checksums = true;
+static bool manifest_with_checksums = false;
 
 static bool success = false;
 static bool made_new_pgdata = false;
@@ -398,6 +399,8 @@ usage(void)
 	printf(_("      --no-slot          prevent creation of temporary replication slot\n"));
 	printf(_("      --no-verify-checksums\n"
 			 "                         do not verify checksums\n"));
+	printf(_("      --manifest-with-checksums\n"
+			 "                         do calculate checksums for manifest files\n"));
 	printf(_("  -?, --help             show this help, then exit\n"));
 	printf(_("\nConnection options:\n"));
 	printf(_("  -d, --dbname=CONNSTR   connection string\n"));
@@ -1821,7 +1824,7 @@ BaseBackup(void)
 	}
 
 	basebkp =
-		psprintf("BASE_BACKUP LABEL '%s' %s %s %s %s %s %s %s",
+		psprintf("BASE_BACKUP LABEL '%s' %s %s %s %s %s %s %s %s",
 				 escaped_label,
 				 showprogress ? "PROGRESS" : "",
 				 includewal == FETCH_WAL ? "WAL" : "",
@@ -1829,7 +1832,8 @@ BaseBackup(void)
 				 includewal == NO_WAL ? "" : "NOWAIT",
 				 maxrate_clause ? maxrate_clause : "",
 				 format == 't' ? "TABLESPACE_MAP" : "",
-				 verify_checksums ? "" : "NOVERIFY_CHECKSUMS");
+				 verify_checksums ? "" : "NOVERIFY_CHECKSUMS",
+				 manifest_with_checksums ? "MANIFEST_WITH_CHECKSUMS" : "");
 
 	if (PQsendQuery(conn, basebkp) == 0)
 	{
@@ -2162,6 +2166,7 @@ main(int argc, char **argv)
 		{"waldir", required_argument, NULL, 1},
 		{"no-slot", no_argument, NULL, 2},
 		{"no-verify-checksums", no_argument, NULL, 3},
+		{"manifest-with-checksums", no_argument, NULL, 4},
 		{NULL, 0, NULL, 0}
 	};
 	int			c;
@@ -2330,6 +2335,9 @@ main(int argc, char **argv)
 			case 3:
 				verify_checksums = false;
 				break;
+			case 4:
+				manifest_with_checksums = true;
+				break;
 			default:
 
 				/*
-- 
1.8.3.1