On Mon, Dec 9, 2019 at 2:52 PM Rushabh Lathia <rushabh.lat...@gmail.com>
wrote:

>
> Thanks Jeevan for reviewing the patch and offline discussion.
>
> On Mon, Dec 9, 2019 at 11:15 AM Jeevan Chalke <
> jeevan.cha...@enterprisedb.com> wrote:
>
>>
>>
>> On Fri, Dec 6, 2019 at 12:05 PM Rushabh Lathia <rushabh.lat...@gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Fri, Dec 6, 2019 at 1:44 AM Robert Haas <robertmh...@gmail.com>
>>> wrote:
>>>
>>>> On Thu, Dec 5, 2019 at 11:22 AM Rushabh Lathia <
>>>> rushabh.lat...@gmail.com> wrote:
>>>> > Here is the whole stack of patches.
>>>>
>>>> I committed 0001, as that's just refactoring and I think (hope) it's
>>>> uncontroversial. I think 0002-0005 need to be squashed together
>>>> (crediting all authors properly and in the appropriate order) as it's
>>>> quite hard to understand right now,
>>>
>>>
>>> Please find attached single patch and I tried to add the credit to all
>>> the authors.
>>>
>>
>> I had a look over the patch and here are my few review comments:
>>
>> 1.
>> +            if (pg_strcasecmp(manifest_checksum_algo, "SHA256") == 0)
>> +                manifest_checksums = MC_SHA256;
>> +            else if (pg_strcasecmp(manifest_checksum_algo, "CRC32C") ==
>> 0)
>> +                manifest_checksums = MC_CRC32C;
>> +            else if (pg_strcasecmp(manifest_checksum_algo, "NONE") == 0)
>> +                manifest_checksums = MC_NONE;
>> +            else
>> +                ereport(ERROR,
>>
>> Is NONE is a valid input? I think the default is "NONE" only and thus no
>> need
>> of this as an input. It will be better if we simply error out if input is
>> neither "SHA256" nor "CRC32C".
>>
>> I believe you have done this way as from pg_basebackup you are always
>> passing
>> MANIFEST_CHECKSUMS '%s' string which will have "NONE" if no user input is
>> given. But I think passing that conditional will be better like we have
>> maxrate_clause for example.
>>
>> Well, this is what I think, feel free to ignore as I don't see any
>> correctness
>> issue over here.
>>
>>
> I would still keep this NONE as it's look more cleaner in the say of
> given options to the checksums.
>
>
>> 2.
>> +    if (manifest_checksums != MC_NONE)
>> +    {
>> +        checksumbuflen = finalize_manifest_checksum(cCtx, checksumbuf);
>> +        switch (manifest_checksums)
>> +        {
>> +            case MC_NONE:
>> +                break;
>> +        }
>>
>> Since switch case is within "if (manifest_checksums != MC_NONE)"
>> condition,
>> I don't think we need a case for MC_NONE here. Rather we can use a default
>> case to error out.
>>
>>
> Yeah, with the new patch we don't have this part of code.
>
>
>> 3.
>> +    if (manifest_checksums != MC_NONE)
>> +    {
>> +        initialize_manifest_checksum(&cCtx);
>> +        update_manifest_checksum(&cCtx, content, len);
>> +    }
>>
>> @@ -1384,6 +1641,9 @@ sendFile(const char *readfilename, const char
>> *tarfilename, struct stat *statbuf
>>      int            segmentno = 0;
>>      char       *segmentpath;
>>      bool        verify_checksum = false;
>> +    ChecksumCtx cCtx;
>> +
>> +    initialize_manifest_checksum(&cCtx);
>>
>>
>> I see that in a few cases you are calling
>> initialize/update_manifest_checksum()
>> conditional and at some other places call is unconditional. It seems like
>> calling unconditional will not have any issues as switch cases inside them
>> return doing nothing when manifest_checksums is MC_NONE.
>>
>>
> Fixed.
>
>
>> 4.
>> initialize/update/finalize_manifest_checksum() functions may be needed by
>> the
>> validation patch as well. And thus I think these functions should not
>> depend
>> on a global variable as such. Also, it will be good if we keep them in a
>> file
>> that is accessible to frontend-only code. Well, you can ignore these
>> comments
>> with the argument saying that this refactoring can be done by the patch
>> adding
>> validation support. I have no issues. Since both the patches are
>> dependent and
>> posted on the same email chain, thought of putting that observation.
>>
>>
> Make sense, I just changed those API to that it doesn't have to
> access the global.
>
>
>> 5.
>> +        switch (manifest_checksums)
>> +        {
>> +            case MC_SHA256:
>> +                checksumlabel = "SHA256:";
>> +                break;
>> +            case MC_CRC32C:
>> +                checksumlabel = "CRC32C:";
>> +                break;
>> +            case MC_NONE:
>> +                break;
>> +        }
>>
>> This code in AddFileToManifest() is executed for every file for which we
>> are
>> adding an entry. However, the checksumlabel will be going to remain the
>> same
>> throughout. Can it be set just once and then used as is?
>>
>>
> Yeah, with the attached patch we no more have this part of code.
>
>
>> 6.
>> Can we avoid manifest_checksums from declaring it as a global variable?
>> I think for that, we need to pass that to every function and thus need to
>> change the function signature of various functions. Currently, we pass
>> "StringInfo manifest" to all the required function, will it better to pass
>> the struct variable instead? A struct may have members like,
>> "StringInfo manifest" in it, checksum type (manifest_checksums),
>> checksum label, etc.
>>
>>
> I agree.  Earlier I was not sure about this because that require data
> structure
> to expose.  But in the given attached patch that's what I tried,
> introduced new
> data structure and defined in basebackup.h and passed the same through the
> function so that doesn't require to pass an individual members.   Also
> removed
> global manifest_checksum and added the same in the newly introduced
> structure.
>
> Attaching the patch, which need to apply on the top of earlier 0001 patch.
>

Attaching another version of 0002 patch, as my collogue Jeevan Chalke
pointed
few indentation problem in 0002 patch which I sent earlier.  Fixed the same
in
the latest patch.




> Thanks,
>
> --
> Rushabh Lathia
> www.EnterpriseDB.com
>


-- 
Rushabh Lathia
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index b777d1f..a5c4a41 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -19,7 +19,6 @@
 #include "access/xlog_internal.h"	/* for pg_start/stop_backup */
 #include "catalog/pg_type.h"
 #include "common/file_perm.h"
-#include "common/sha2.h"
 #include "lib/stringinfo.h"
 #include "libpq/libpq.h"
 #include "libpq/pqformat.h"
@@ -52,42 +51,28 @@ typedef struct
 	bool		includewal;
 	uint32		maxrate;
 	bool		sendtblspcmapfile;
+	ManifestCheckSum checksumAlgo;
 } basebackup_options;
 
-/* Checksum algorithm option for manifest */
-enum manifestCheckSum
-{
-	MC_NONE = 0,
-	MC_SHA256,
-	MC_CRC32C
-};
-
-/* checksum algorithm context */
-typedef union checksumCtx
-{
-	pg_sha256_ctx	sha256_ctx;
-	pg_crc32c		crc_ctx;
-}	ChecksumCtx;
-
 static int64 sendDir(const char *path, int basepathlen, bool sizeonly,
 					 List *tablespaces, bool sendtblspclinks,
-					 StringInfo manifest, const char *tsoid);
+					 manifestinfo *manifestInfo, const char *tsoid);
 static bool sendFile(const char *readfilename, const char *tarfilename,
 					 struct stat *statbuf, bool missing_ok, Oid dboid,
-					 StringInfo manifest, const char *tsoid);
+					 manifestinfo *manifestInfo, const char *tsoid);
 static void sendFileWithContent(const char *filename, const char *content,
-								StringInfo manifest);
+								manifestinfo *manifestInfo);
 static int64 _tarWriteHeader(const char *filename, const char *linktarget,
 							 struct stat *statbuf, bool sizeonly);
 static int64 _tarWriteDir(const char *pathbuf, int basepathlen, struct stat *statbuf,
 						  bool sizeonly);
 static void send_int8_string(StringInfoData *buf, int64 intval);
 static void SendBackupHeader(List *tablespaces);
-static void InitializeManifest(StringInfo manifest);
-static void AddFileToManifest(StringInfo manifest, const char *tsoid,
-							  const char *filename, size_t size, time_t mtime,
-							  ChecksumCtx *cCtx);
-static void SendBackupManifest(StringInfo manifest);
+static void InitializeManifest(manifestinfo *manifestInfo,
+							   ManifestCheckSum checksumAlgo);
+static void AddFileToManifest(manifestinfo *manifestInfo, const char *tsoid,
+							  const char *filename, size_t size, time_t mtime);
+static void SendBackupManifest(manifestinfo *manifestInfo);
 static char *escape_field_for_manifest(const char *s);
 static void base_backup_cleanup(int code, Datum arg);
 static void perform_base_backup(basebackup_options *opt);
@@ -96,9 +81,14 @@ static void SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli);
 static int	compareWalFileNames(const ListCell *a, const ListCell *b);
 static void throttle(size_t increment);
 static bool is_checksummed_file(const char *fullpath, const char *filename);
-static void initialize_manifest_checksum(ChecksumCtx *cCtx);
-static void update_manifest_checksum(ChecksumCtx *cCtx, const char *buf, off_t cnt);
-static int finalize_manifest_checksum(ChecksumCtx *cCtx, char *checksumbuf);
+static void initialize_manifest_checksum(ChecksumCtx *cCtx,
+										 ManifestCheckSum checksumAlgo);
+static void update_manifest_checksum(ChecksumCtx *cCtx,
+									 ManifestCheckSum checksumAlgo,
+									 const char *buf, off_t cnt);
+static int finalize_manifest_checksum(ChecksumCtx *cCtx,
+									  ManifestCheckSum checksumAlgo,
+									  char *checksumbuf);
 
 /* Was the backup currently in-progress initiated in recovery mode? */
 static bool backup_started_in_recovery = false;
@@ -150,8 +140,6 @@ static long long int total_checksum_failures;
 static bool noverify_checksums = false;
 
 
-static enum manifestCheckSum manifest_checksums = MC_NONE;
-
 /*
  * The contents of these directories are removed or recreated during server
  * start so they are not included in backups.  The directories themselves are
@@ -271,9 +259,9 @@ perform_base_backup(basebackup_options *opt)
 	TimeLineID	endtli;
 	StringInfo	labelfile;
 	StringInfo	tblspc_map_file = NULL;
-	StringInfo	manifest;
 	int			datadirpathlen;
 	List	   *tablespaces = NIL;
+	manifestinfo manifestInfo;
 
 	datadirpathlen = strlen(DataDir);
 
@@ -281,8 +269,7 @@ perform_base_backup(basebackup_options *opt)
 
 	labelfile = makeStringInfo();
 	tblspc_map_file = makeStringInfo();
-	manifest = makeStringInfo();
-	InitializeManifest(manifest);
+	InitializeManifest(&manifestInfo, opt->checksumAlgo);
 
 	total_checksum_failures = 0;
 
@@ -370,7 +357,7 @@ perform_base_backup(basebackup_options *opt)
 
 				/* In the main tar, include the backup_label first... */
 				sendFileWithContent(BACKUP_LABEL_FILE, labelfile->data,
-									manifest);
+									&manifestInfo);
 
 				/*
 				 * Send tablespace_map file if required and then the bulk of
@@ -379,11 +366,13 @@ perform_base_backup(basebackup_options *opt)
 				if (tblspc_map_file && opt->sendtblspcmapfile)
 				{
 					sendFileWithContent(TABLESPACE_MAP, tblspc_map_file->data,
-										manifest);
-					sendDir(".", 1, false, tablespaces, false, manifest, NULL);
+										&manifestInfo);
+					sendDir(".", 1, false, tablespaces,
+							false, &manifestInfo, NULL);
 				}
 				else
-					sendDir(".", 1, false, tablespaces, true, manifest, NULL);
+					sendDir(".", 1, false, tablespaces,
+							true, &manifestInfo, NULL);
 
 				/* ... and pg_control after everything else. */
 				if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0)
@@ -392,10 +381,10 @@ perform_base_backup(basebackup_options *opt)
 							 errmsg("could not stat file \"%s\": %m",
 									XLOG_CONTROL_FILE)));
 				sendFile(XLOG_CONTROL_FILE, XLOG_CONTROL_FILE, &statbuf,
-						 false, InvalidOid, manifest, NULL);
+						 false, InvalidOid, &manifestInfo, NULL);
 			}
 			else
-				sendTablespace(ti->path, ti->oid, false, manifest);
+				sendTablespace(ti->path, ti->oid, false, &manifestInfo);
 
 			/*
 			 * If we're including WAL, and this is the main data directory we
@@ -614,7 +603,7 @@ perform_base_backup(basebackup_options *opt)
 			 * complete segment.
 			 */
 			StatusFilePath(pathbuf, walFileName, ".done");
-			sendFileWithContent(pathbuf, "", manifest);
+			sendFileWithContent(pathbuf, "", &manifestInfo);
 		}
 
 		/*
@@ -637,19 +626,19 @@ perform_base_backup(basebackup_options *opt)
 						(errcode_for_file_access(),
 						 errmsg("could not stat file \"%s\": %m", pathbuf)));
 
-			sendFile(pathbuf, pathbuf, &statbuf, false, InvalidOid, manifest,
-					 NULL);
+			sendFile(pathbuf, pathbuf, &statbuf, false, InvalidOid,
+					 &manifestInfo, NULL);
 
 			/* unconditionally mark file as archived */
 			StatusFilePath(pathbuf, fname, ".done");
-			sendFileWithContent(pathbuf, "", manifest);
+			sendFileWithContent(pathbuf, "", &manifestInfo);
 		}
 
 		/* Send CopyDone message for the last tar file */
 		pq_putemptymessage('c');
 	}
 
-	SendBackupManifest(manifest);
+	SendBackupManifest(&manifestInfo);
 
 	SendXlogRecPtrResult(endptr, endtli);
 
@@ -793,11 +782,11 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 			manifest_checksum_algo = strVal(defel->arg);
 
 			if (pg_strcasecmp(manifest_checksum_algo, "SHA256") == 0)
-				manifest_checksums = MC_SHA256;
+				opt->checksumAlgo = MC_SHA256;
 			else if (pg_strcasecmp(manifest_checksum_algo, "CRC32C") == 0)
-				manifest_checksums = MC_CRC32C;
+				opt->checksumAlgo = MC_CRC32C;
 			else if (pg_strcasecmp(manifest_checksum_algo, "NONE") == 0)
-				manifest_checksums = MC_NONE;
+				opt->checksumAlgo = MC_NONE;
 			else
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
@@ -926,18 +915,35 @@ SendBackupHeader(List *tablespaces)
 }
 
 static void
-InitializeManifest(StringInfo manifest)
+InitializeManifest(manifestinfo *manifestInfo, ManifestCheckSum checksumAlgo)
 {
-	appendStringInfoString(manifest, "PostgreSQL-Backup-Manifest-Version 1\n");
+	Assert(manifestInfo != NULL);
+
+	MemSet(manifestInfo, 0, sizeof(*manifestInfo));
+
+	manifestInfo->checksumAlgo = checksumAlgo;
+	manifestInfo->manifest = makeStringInfo();
+	appendStringInfoString(manifestInfo->manifest, "PostgreSQL-Backup-Manifest-Version 1\n");
+
+	switch (manifestInfo->checksumAlgo)
+	{
+		case MC_SHA256:
+			strcpy(manifestInfo->checksum_label, "SHA256:");
+			break;
+		case MC_CRC32C:
+			strcpy(manifestInfo->checksum_label, "CRC32C:");
+			break;
+		case MC_NONE:
+			break;
+	}
 }
 
 /*
  * Add an entry to the backup manifest for a file.
  */
 static void
-AddFileToManifest(StringInfo manifest, const char *tsoid,
-				  const char *filename, size_t size, time_t mtime,
-				  ChecksumCtx *cCtx)
+AddFileToManifest(manifestinfo *manifestInfo, const char *tsoid,
+				  const char *filename, size_t size, time_t mtime)
 {
 	char	pathbuf[MAXPGPATH];
 	char   *escaped_filename;
@@ -945,8 +951,10 @@ AddFileToManifest(StringInfo manifest, const char *tsoid,
 	static char checksumbuf[256];
 	char encode_checksumbuf[256];
 	struct pg_tm *tm;
-	char *checksumlabel = NULL;
+	char *checksumlabel = manifestInfo->checksum_label;
 	int	   checksumbuflen;
+	ChecksumCtx *cCtx = &manifestInfo->cCtx;
+	StringInfo manifest = manifestInfo->manifest;
 
 	/*
 	 * If this file is part of a tablespace, the filename passed to this
@@ -974,20 +982,10 @@ AddFileToManifest(StringInfo manifest, const char *tsoid,
 	pg_strftime(timebuf, sizeof(timebuf), "%Y-%m-%d %H:%M:%S %Z", tm);
 
 	/* Generate final checksum and Convert it to hexadecimal. */
-	if (manifest_checksums != MC_NONE)
+	if (manifestInfo->checksumAlgo != MC_NONE)
 	{
-		checksumbuflen = finalize_manifest_checksum(cCtx, checksumbuf);
-		switch (manifest_checksums)
-		{
-			case MC_SHA256:
-				checksumlabel = "SHA256:";
-				break;
-			case MC_CRC32C:
-				checksumlabel = "CRC32C:";
-				break;
-			case MC_NONE:
-				break;
-		}
+		checksumbuflen = finalize_manifest_checksum(cCtx,
+								manifestInfo->checksumAlgo, checksumbuf);
 		checksumbuflen = hex_encode(checksumbuf,
 									checksumbuflen,
 									encode_checksumbuf);
@@ -998,7 +996,7 @@ AddFileToManifest(StringInfo manifest, const char *tsoid,
 	appendStringInfo(manifest, "File\t%s\t%zu\t%s\t%s%s\n",
 					 escaped_filename == NULL ? filename : escaped_filename,
 					 size, timebuf, checksumlabel ? checksumlabel : "",
-					 manifest_checksums != MC_NONE ? encode_checksumbuf : "-");
+					 manifestInfo->checksumAlgo != MC_NONE ? encode_checksumbuf : "-");
 
 	/* Avoid leaking memory. */
 	if (escaped_filename != NULL)
@@ -1009,31 +1007,25 @@ AddFileToManifest(StringInfo manifest, const char *tsoid,
  * Finalize the backup manifest, and send it to the client.
  */
 static void
-SendBackupManifest(StringInfo manifest)
+SendBackupManifest(manifestinfo *manifestInfo)
 {
 	char			checksumbuf[256];
 	StringInfoData	protobuf;
 	int				checksumbuflen;
-	ChecksumCtx		cCtx;
+	ChecksumCtx	   *cCtx = &manifestInfo->cCtx;
+	StringInfo		manifest = manifestInfo->manifest;
 
 	/* Checksum the manifest. */
-	if (manifest_checksums != MC_NONE)
+	if (manifestInfo->checksumAlgo != MC_NONE)
 	{
-		initialize_manifest_checksum(&cCtx);
-		update_manifest_checksum(&cCtx, manifest->data, manifest->len);
-		checksumbuflen = finalize_manifest_checksum(&cCtx, (char *) checksumbuf);
+		initialize_manifest_checksum(cCtx, manifestInfo->checksumAlgo);
+		update_manifest_checksum(cCtx, manifestInfo->checksumAlgo,
+								 manifest->data, manifest->len);
+		checksumbuflen = finalize_manifest_checksum(cCtx,
+										manifestInfo->checksumAlgo,
+										(char *) checksumbuf);
 		appendStringInfoString(manifest, "Manifest-Checksum\t");
-		switch (manifest_checksums)
-		{
-			case MC_SHA256:
-				appendStringInfoString(manifest, "SHA256:");
-				break;
-			case MC_CRC32C:
-				appendStringInfoString(manifest, "CRC32C:");
-				break;
-			case MC_NONE:
-				break;
-		}
+		appendStringInfoString(manifest, manifestInfo->checksum_label);
 		enlargeStringInfo(manifest, checksumbuflen * 2);
 		checksumbuflen = hex_encode(checksumbuf, checksumbuflen,
 				manifest->data + manifest->len);
@@ -1164,12 +1156,11 @@ SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli)
  */
 static void
 sendFileWithContent(const char *filename, const char *content,
-					StringInfo manifest)
+					manifestinfo *manifestInfo)
 {
 	struct stat statbuf;
 	int			pad,
 				len;
-	ChecksumCtx cCtx;
 
 	len = strlen(content);
 
@@ -1203,14 +1194,11 @@ sendFileWithContent(const char *filename, const char *content,
 		pq_putmessage('d', buf, pad);
 	}
 
-	if (manifest_checksums != MC_NONE)
-	{
-		initialize_manifest_checksum(&cCtx);
-		update_manifest_checksum(&cCtx, content, len);
-	}
-
-	AddFileToManifest(manifest, NULL, filename, len, statbuf.st_mtime,
-					  &cCtx);
+	initialize_manifest_checksum(&manifestInfo->cCtx,
+								 manifestInfo->checksumAlgo);
+	update_manifest_checksum(&manifestInfo->cCtx, manifestInfo->checksumAlgo,
+							 content, len);
+	AddFileToManifest(manifestInfo, NULL, filename, len, statbuf.st_mtime);
 }
 
 /*
@@ -1221,7 +1209,7 @@ sendFileWithContent(const char *filename, const char *content,
  * Only used to send auxiliary tablespaces, not PGDATA.
  */
 int64
-sendTablespace(char *path, char *oid, bool sizeonly, StringInfo manifest)
+sendTablespace(char *path, char *oid, bool sizeonly, manifestinfo *manifestInfo)
 {
 	int64		size;
 	char		pathbuf[MAXPGPATH];
@@ -1254,7 +1242,8 @@ sendTablespace(char *path, char *oid, bool sizeonly, StringInfo manifest)
 						   sizeonly);
 
 	/* Send all the files in the tablespace version directory */
-	size += sendDir(pathbuf, strlen(path), sizeonly, NIL, true, manifest, oid);
+	size += sendDir(pathbuf, strlen(path), sizeonly, NIL,
+					true, manifestInfo, oid);
 
 	return size;
 }
@@ -1273,7 +1262,7 @@ sendTablespace(char *path, char *oid, bool sizeonly, StringInfo manifest)
  */
 static int64
 sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
-		bool sendtblspclinks, StringInfo manifest, const char *tsoid)
+		bool sendtblspclinks, manifestinfo *manifestInfo, const char *tsoid)
 {
 	DIR		   *dir;
 	struct dirent *de;
@@ -1550,7 +1539,7 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
 
 			if (!skip_this_dir)
 				size += sendDir(pathbuf, basepathlen, sizeonly, tablespaces,
-								sendtblspclinks, manifest, tsoid);
+								sendtblspclinks, manifestInfo, tsoid);
 		}
 		else if (S_ISREG(statbuf.st_mode))
 		{
@@ -1559,7 +1548,7 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
 			if (!sizeonly)
 				sent = sendFile(pathbuf, pathbuf + basepathlen + 1, &statbuf,
 								true, isDbDir ? pg_atoi(lastDir + 1, sizeof(Oid), 0) : InvalidOid,
-								manifest, tsoid);
+								manifestInfo, tsoid);
 
 			if (sent || sizeonly)
 			{
@@ -1624,7 +1613,7 @@ is_checksummed_file(const char *fullpath, const char *filename)
 static bool
 sendFile(const char *readfilename, const char *tarfilename,
 		 struct stat *statbuf, bool missing_ok, Oid dboid,
-		 StringInfo manifest, const char *tsoid)
+		 manifestinfo *manifestInfo, const char *tsoid)
 {
 	FILE	   *fp;
 	BlockNumber blkno = 0;
@@ -1641,9 +1630,9 @@ sendFile(const char *readfilename, const char *tarfilename,
 	int			segmentno = 0;
 	char	   *segmentpath;
 	bool		verify_checksum = false;
-	ChecksumCtx cCtx;
 
-	initialize_manifest_checksum(&cCtx);
+	initialize_manifest_checksum(&manifestInfo->cCtx,
+								 manifestInfo->checksumAlgo);
 
 	fp = AllocateFile(readfilename, "rb");
 	if (fp == NULL)
@@ -1814,7 +1803,8 @@ sendFile(const char *readfilename, const char *tarfilename,
 					(errmsg("base backup could not send data, aborting backup")));
 
 		/* Also feed it to the checksum machinery. */
-		update_manifest_checksum(&cCtx, buf, cnt);
+		update_manifest_checksum(&manifestInfo->cCtx, manifestInfo->checksumAlgo,
+								 buf, cnt);
 
 		len += cnt;
 		throttle(cnt);
@@ -1840,7 +1830,9 @@ sendFile(const char *readfilename, const char *tarfilename,
 		{
 			cnt = Min(sizeof(buf), statbuf->st_size - len);
 			pq_putmessage('d', buf, cnt);
-			update_manifest_checksum(&cCtx, buf, cnt);
+			update_manifest_checksum(&manifestInfo->cCtx,
+									 manifestInfo->checksumAlgo,
+									 buf, cnt);
 			len += cnt;
 			throttle(cnt);
 		}
@@ -1872,8 +1864,8 @@ sendFile(const char *readfilename, const char *tarfilename,
 	}
 
 	total_checksum_failures += checksum_failures;
-	AddFileToManifest(manifest, tsoid, tarfilename, statbuf->st_size,
-					  statbuf->st_mtime, &cCtx);
+	AddFileToManifest(manifestInfo, tsoid, tarfilename, statbuf->st_size,
+					  statbuf->st_mtime);
 
 	return true;
 }
@@ -2014,9 +2006,9 @@ throttle(size_t increment)
  * Initialize the manifest checksum context according to the provided algorithm.
  */
 static void
-initialize_manifest_checksum(ChecksumCtx *cCtx)
+initialize_manifest_checksum(ChecksumCtx *cCtx, ManifestCheckSum checksumAlgo)
 {
-	switch (manifest_checksums)
+	switch (checksumAlgo)
 	{
 		case MC_SHA256:
 			pg_sha256_init(&cCtx->sha256_ctx);
@@ -2030,9 +2022,10 @@ initialize_manifest_checksum(ChecksumCtx *cCtx)
 }
 
 static void
-update_manifest_checksum(ChecksumCtx *cCtx, const char *buf, off_t cnt)
+update_manifest_checksum(ChecksumCtx *cCtx, ManifestCheckSum checksumAlgo,
+						 const char *buf, off_t cnt)
 {
-	switch (manifest_checksums)
+	switch (checksumAlgo)
 	{
 		case MC_SHA256:
 			pg_sha256_update(&cCtx->sha256_ctx, (uint8 *) buf, cnt);
@@ -2050,10 +2043,12 @@ update_manifest_checksum(ChecksumCtx *cCtx, const char *buf, off_t cnt)
  * the length of checksum.
  */
 static int
-finalize_manifest_checksum(ChecksumCtx *cCtx, char *checksumbuf)
+finalize_manifest_checksum(ChecksumCtx *cCtx,
+						   ManifestCheckSum checksumAlgo,
+						   char *checksumbuf)
 {
 	int checksumlen = 0;
-	switch (manifest_checksums)
+	switch (checksumAlgo)
 	{
 		case MC_SHA256:
 			pg_sha256_final(&cCtx->sha256_ctx, (uint8 *)checksumbuf);
diff --git a/src/include/replication/basebackup.h b/src/include/replication/basebackup.h
index 8fe0136..0b165e88 100644
--- a/src/include/replication/basebackup.h
+++ b/src/include/replication/basebackup.h
@@ -12,6 +12,7 @@
 #ifndef _BASEBACKUP_H
 #define _BASEBACKUP_H
 
+#include "common/sha2.h"
 #include "lib/stringinfo.h"
 #include "nodes/replnodes.h"
 
@@ -30,9 +31,33 @@ typedef struct
 	int64		size;
 } tablespaceinfo;
 
+/* Checksum algorithm option for manifest */
+typedef enum ManifestCheckSum
+{
+	MC_NONE = 0,
+	MC_SHA256,
+	MC_CRC32C
+} ManifestCheckSum;
+
+/* checksum algorithm context */
+typedef union checksumCtx
+{
+	pg_sha256_ctx sha256_ctx;
+	pg_crc32c	crc_ctx;
+} ChecksumCtx;
+
+/* Backup manifest info */
+typedef struct
+{
+	ManifestCheckSum checksumAlgo;
+	char		checksum_label[10];
+	ChecksumCtx cCtx;
+	StringInfo	manifest;
+} manifestinfo;
+
 extern void SendBaseBackup(BaseBackupCmd *cmd);
 
 extern int64 sendTablespace(char *path, char *oid, bool sizeonly,
-							StringInfo manifest);
+							manifestinfo * manifestInfo);
 
 #endif							/* _BASEBACKUP_H */

Reply via email to