From bf7009577230ed671d70d736bd59ac30f54d5439 Mon Sep 17 00:00:00 2001 From: Rushabh Lathia Date: Fri, 22 Nov 2019 15:17:47 +0530 Subject: [PATCH 3/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 command %type 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