v2-0009-WIP-Convert-backup-manifest-generation-to-a-bbarc.patch

From 6b41e7244e136325872a6bbeccbde2a13ddc79cc Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Fri, 8 May 2020 15:21:01 -0400
Subject: [PATCH v2 09/11] WIP: Convert backup manifest generation to a
 bbarchiver.

---
 src/backend/replication/backup_manifest.c | 106 +++++++++++++++++++-
 src/backend/replication/basebackup.c      | 113 +++++++++-------------
 src/include/replication/backup_manifest.h |   5 +-
 3 files changed, 153 insertions(+), 71 deletions(-)

diff --git a/src/backend/replication/backup_manifest.c b/src/backend/replication/backup_manifest.c
index ff326bce19..733c4d6313 100644
--- a/src/backend/replication/backup_manifest.c
+++ b/src/backend/replication/backup_manifest.c
@@ -17,7 +17,6 @@
 #include "libpq/pqformat.h"
 #include "mb/pg_wchar.h"
 #include "replication/backup_manifest.h"
-#include "replication/basebackup_sink.h"
 #include "utils/builtins.h"
 #include "utils/json.h"
 
@@ -366,3 +365,108 @@ AppendStringToManifest(backup_manifest_info *manifest, char *s)
 	BufFileWrite(manifest->buffile, s, len);
 	manifest->manifest_size += len;
 }
+
+typedef struct bbarchiver_manifest
+{
+	bbarchiver	base;
+
+	/* These things are fixed at creation time. */
+	backup_manifest_info *manifest;
+
+	/* This changes for each tablespace. */
+	const char *spcoid;
+
+	/* These change for each file. */
+	const char *file_pathname;
+	size_t		file_size;
+	pg_time_t	file_mtime;
+	pg_checksum_context file_checksum_ctx;
+} bbarchiver_manifest;
+
+static void bbarchiver_manifest_begin_tablespace(bbarchiver *archiver,
+												 tablespaceinfo *tsinfo);
+static void bbarchiver_manifest_begin_file(bbarchiver *archiver,
+									  const char *relative_path,
+									  struct stat *statbuf);
+static void bbarchiver_manifest_file_contents(bbarchiver *archiver,
+										 const char *data,
+										 size_t len);
+static void bbarchiver_manifest_end_file(bbarchiver *archiver);
+
+static const bbarchiver_ops bbarchiver_manifest_ops = {
+	.begin_tablespace = bbarchiver_manifest_begin_tablespace,
+	.end_tablespace = bbarchiver_forward_end_tablespace,
+	.begin_file = bbarchiver_manifest_begin_file,
+	.file_contents = bbarchiver_manifest_file_contents,
+	.end_file = bbarchiver_manifest_end_file,
+	.directory = bbarchiver_forward_directory,
+	.symbolic_link = bbarchiver_forward_symbolic_link
+};
+
+extern bbarchiver *
+bbarchiver_manifest_new(bbarchiver *next, backup_manifest_info *manifest)
+{
+	bbarchiver_manifest *archiver = palloc0(sizeof(bbarchiver_manifest));
+
+	*((const bbarchiver_ops **) &archiver->base.bba_ops) =
+		&bbarchiver_manifest_ops;
+	archiver->base.bba_next = next;
+	archiver->manifest = manifest;
+
+	return &archiver->base;
+}
+
+static void
+bbarchiver_manifest_begin_tablespace(bbarchiver *archiver,
+									 tablespaceinfo *tsinfo)
+{
+	bbarchiver_manifest *myarchiver = (bbarchiver_manifest *) archiver;
+
+	myarchiver->spcoid = tsinfo->oid;
+
+	bbarchiver_begin_tablespace(archiver->bba_next, tsinfo);
+}
+
+static void
+bbarchiver_manifest_begin_file(bbarchiver *archiver,
+							   const char *relative_path,
+							   struct stat *statbuf)
+{
+	bbarchiver_manifest *myarchiver = (bbarchiver_manifest *) archiver;
+
+	myarchiver->file_pathname = relative_path;
+	myarchiver->file_size = statbuf->st_size;
+	myarchiver->file_mtime = statbuf->st_mtime;
+
+	pg_checksum_init(&myarchiver->file_checksum_ctx,
+					 myarchiver->manifest->checksum_type);
+
+	bbarchiver_begin_file(archiver->bba_next, relative_path, statbuf);
+}
+
+static void
+bbarchiver_manifest_file_contents(bbarchiver *archiver,
+								  const char *data, size_t len)
+{
+	bbarchiver_manifest *myarchiver = (bbarchiver_manifest *) archiver;
+
+	pg_checksum_update(&myarchiver->file_checksum_ctx,
+					   (uint8 *) data, len);
+
+	bbarchiver_file_contents(archiver->bba_next, data, len);
+}
+
+static void
+bbarchiver_manifest_end_file(bbarchiver *archiver)
+{
+	bbarchiver_manifest *myarchiver = (bbarchiver_manifest *) archiver;
+
+	AddFileToBackupManifest(myarchiver->manifest,
+							myarchiver->spcoid,
+							myarchiver->file_pathname,
+							myarchiver->file_size,
+							myarchiver->file_mtime,
+							&myarchiver->file_checksum_ctx);
+
+	bbarchiver_end_file(archiver->bba_next);
+}
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index e8183daa68..30e242f99e 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -59,23 +59,17 @@ typedef struct
 static void archive_database_cluster(List *tablespaces, bbarchiver *archiver,
 									 StringInfo labelfile,
 									 StringInfo tblspc_map_file,
-									 bool leave_main_tablespace_open,
-									 backup_manifest_info *manifest);
-static void archive_tablespace(bbarchiver *archiver, char *path, char *oid,
-							   struct backup_manifest_info *manifest);
+									 bool leave_main_tablespace_open);
+static void archive_tablespace(bbarchiver *archiver, char *path);
 static void archive_directory(bbarchiver *archiver, const char *path,
 							  int basepathlen, List *tablespaces,
-							  bool sendtblspclinks,
-							  backup_manifest_info *manifest,
-							  const char *spcoid);
+							  bool sendtblspclinks);
 static void archive_file(bbarchiver *archiver, const char *readfilename,
 						 const char *tarfilename, struct stat *statbuf,
-						 bool missing_ok, Oid dboid,
-						 backup_manifest_info *manifest, const char *spcoid);
+						 bool missing_ok, Oid dboid);
 static void archive_file_with_content(bbarchiver *archiver,
 									  const char *filename,
-									  const char *content,
-									  backup_manifest_info *manifest);
+									  const char *content);
 static void convert_link_to_directory(const char *pathbuf, struct stat *statbuf);
 static void perform_base_backup(basebackup_options *opt);
 static void parse_basebackup_options(List *options, basebackup_options *opt);
@@ -242,6 +236,14 @@ perform_base_backup(basebackup_options *opt)
 	bbarchiver *archiver;
 	bbarchiver *size_archiver;
 
+	/*
+	 * The backup manifest code uses a BufFile, so create a ResourceOwner.
+	 * This is cheap enough that we don't worry about doing it only if it's
+	 * needed, and there might be other uses for it in the future.
+	 */
+	Assert(CurrentResourceOwner == NULL);
+	CurrentResourceOwner = ResourceOwnerCreate(NULL, "base backup");
+
 	/* Set up network throttling, if client requested it */
 	if (opt->maxrate > 0)
 		sink = bbsink_throttle_new(sink, opt->maxrate);
@@ -253,9 +255,13 @@ perform_base_backup(basebackup_options *opt)
 	archiver = bbarchiver_tar_new(sink);
 	size_archiver = bbarchiver_tarsize_new();
 
-	/* we're going to use a BufFile, so we need a ResourceOwner */
-	Assert(CurrentResourceOwner == NULL);
-	CurrentResourceOwner = ResourceOwnerCreate(NULL, "base backup");
+	/* Set up backup manifest gneration, if enabled. */
+	if (opt->manifest != MANIFEST_OPTION_NO)
+	{
+		InitializeBackupManifest(&manifest, opt->manifest,
+								 opt->manifest_checksum_type);
+		archiver = bbarchiver_manifest_new(archiver, &manifest);
+	}
 
 	datadirpathlen = strlen(DataDir);
 
@@ -263,8 +269,6 @@ perform_base_backup(basebackup_options *opt)
 
 	labelfile = makeStringInfo();
 	tblspc_map_file = makeStringInfo();
-	InitializeBackupManifest(&manifest, opt->manifest,
-							 opt->manifest_checksum_type);
 
 	total_checksum_failures = 0;
 
@@ -309,7 +313,7 @@ perform_base_backup(basebackup_options *opt)
 			basebackup_progress_estimate_backup_size();
 
 			archive_database_cluster(tablespaces, size_archiver, labelfile,
-									 tblspc_map_file, false, NULL);
+									 tblspc_map_file, false);
 		}
 
 		/* notify basebackup sink about start of backup */
@@ -322,7 +326,7 @@ perform_base_backup(basebackup_options *opt)
 		 * so that we can archive the WAL files as well.
 		 */
 		archive_database_cluster(tablespaces, archiver, labelfile,
-								 tblspc_map_file, opt->includewal, &manifest);
+								 tblspc_map_file, opt->includewal);
 
 		basebackup_progress_wait_wal_archive(progress_sink);
 		endptr = do_pg_stop_backup(labelfile->data, !opt->nowait, &endtli);
@@ -524,7 +528,7 @@ perform_base_backup(basebackup_options *opt)
 			 * complete segment.
 			 */
 			StatusFilePath(pathbuf, walFileName, ".done");
-			archive_file_with_content(archiver, pathbuf, "", &manifest);
+			archive_file_with_content(archiver, pathbuf, "");
 		}
 
 		/*
@@ -548,19 +552,23 @@ perform_base_backup(basebackup_options *opt)
 						 errmsg("could not stat file \"%s\": %m", pathbuf)));
 
 			archive_file(archiver, pathbuf, pathbuf, &statbuf, false,
-						 InvalidOid, &manifest, NULL);
+						 InvalidOid);
 
 			/* unconditionally mark file as archived */
 			StatusFilePath(pathbuf, fname, ".done");
-			archive_file_with_content(archiver, pathbuf, "", &manifest);
+			archive_file_with_content(archiver, pathbuf, "");
 		}
 
 		bbsink_end_archive(sink);
 	}
 
-	AddWALInfoToBackupManifest(&manifest, startptr, starttli, endptr, endtli);
 
-	SendBackupManifest(&manifest, sink);
+	if (opt->manifest != MANIFEST_OPTION_NO)
+	{
+		AddWALInfoToBackupManifest(&manifest, startptr, starttli,
+								   endptr, endtli);
+		SendBackupManifest(&manifest, sink);
+	}
 
 	bbsink_end_backup(sink, endptr, endtli);
 
@@ -588,8 +596,7 @@ perform_base_backup(basebackup_options *opt)
 static void
 archive_database_cluster(List *tablespaces, bbarchiver *archiver,
 						 StringInfo labelfile, StringInfo tblspc_map_file,
-						 bool leave_main_tablespace_open,
-						 backup_manifest_info *manifest)
+						 bool leave_main_tablespace_open)
 {
 	ListCell   *lc;
 
@@ -607,20 +614,18 @@ archive_database_cluster(List *tablespaces, bbarchiver *archiver,
 
 			/* For the main tablespace, archive the backup_label first... */
 			archive_file_with_content(archiver, BACKUP_LABEL_FILE,
-									  labelfile->data, manifest);
+									  labelfile->data);
 
 			/* Then the tablespace_map file, if present... */
 			if (tblspc_map_file != NULL)
 			{
 				archive_file_with_content(archiver, TABLESPACE_MAP,
-										  tblspc_map_file->data,
-										  manifest);
+										  tblspc_map_file->data);
 				sendtblspclinks = false;
 			}
 
 			/* Then the bulk of the files... */
-			archive_directory(archiver, ".", 1, tablespaces,
-							  sendtblspclinks, manifest, NULL);
+			archive_directory(archiver, ".", 1, tablespaces, sendtblspclinks);
 
 			/* ... and pg_control after everything else. */
 			if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0)
@@ -629,10 +634,10 @@ archive_database_cluster(List *tablespaces, bbarchiver *archiver,
 						 errmsg("could not stat file \"%s\": %m",
 								XLOG_CONTROL_FILE)));
 			archive_file(archiver, XLOG_CONTROL_FILE, XLOG_CONTROL_FILE,
-						 &statbuf, false, InvalidOid, manifest, NULL);
+						 &statbuf, false, InvalidOid);
 		}
 		else
-			archive_tablespace(archiver, ti->path, ti->oid, manifest);
+			archive_tablespace(archiver, ti->path);
 
 		/*
 		 * If we were asked to leave the main tablespace open, then do so.
@@ -865,14 +870,10 @@ SendBaseBackup(BaseBackupCmd *cmd)
  */
 static void
 archive_file_with_content(bbarchiver *archiver, const char *filename,
-						  const char *content, backup_manifest_info *manifest)
+						  const char *content)
 {
 	struct stat statbuf;
 	int			len;
-	pg_checksum_context checksum_ctx;
-
-	if (manifest != NULL)
-		pg_checksum_init(&checksum_ctx, manifest->checksum_type);
 
 	len = strlen(content);
 
@@ -896,13 +897,6 @@ archive_file_with_content(bbarchiver *archiver, const char *filename,
 	if (bbarchiver_needs_file_contents(archiver))
 		bbarchiver_file_contents(archiver, content, len);
 	bbarchiver_end_file(archiver);
-
-	if (manifest != NULL)
-	{
-		pg_checksum_update(&checksum_ctx, (uint8 *) content, len);
-		AddFileToBackupManifest(manifest, NULL, filename, len,
-								(pg_time_t) statbuf.st_mtime, &checksum_ctx);
-	}
 }
 
 /*
@@ -913,8 +907,7 @@ archive_file_with_content(bbarchiver *archiver, const char *filename,
  * Only used to send auxiliary tablespaces, not PGDATA.
  */
 static void
-archive_tablespace(bbarchiver *archiver, char *path, char *spcoid,
-				   backup_manifest_info *manifest)
+archive_tablespace(bbarchiver *archiver, char *path)
 {
 	char		pathbuf[MAXPGPATH];
 	struct stat statbuf;
@@ -945,8 +938,7 @@ archive_tablespace(bbarchiver *archiver, char *path, char *spcoid,
 	bbarchiver_directory(archiver, TABLESPACE_VERSION_DIRECTORY, &statbuf);
 
 	/* Send all the files in the tablespace version directory */
-	archive_directory(archiver, pathbuf, strlen(path), NIL, true, manifest,
-					  spcoid);
+	archive_directory(archiver, pathbuf, strlen(path), NIL, true);
 }
 
 /*
@@ -963,8 +955,7 @@ archive_tablespace(bbarchiver *archiver, char *path, char *spcoid,
  */
 static void
 archive_directory(bbarchiver *archiver, const char *path, int basepathlen,
-				  List *tablespaces, bool sendtblspclinks,
-				  backup_manifest_info *manifest, const char *spcoid)
+				  List *tablespaces, bool sendtblspclinks)
 {
 	DIR		   *dir;
 	struct dirent *de;
@@ -1250,14 +1241,13 @@ archive_directory(bbarchiver *archiver, const char *path, int basepathlen,
 
 			if (!skip_this_dir)
 				archive_directory(archiver, pathbuf, basepathlen, tablespaces,
-								  sendtblspclinks, manifest, spcoid);
+								  sendtblspclinks);
 		}
 		else if (S_ISREG(statbuf.st_mode))
 		{
 			archive_file(archiver, pathbuf, pathbuf + basepathlen + 1,
 						 &statbuf, true,
-						 isDbDir ? atooid(lastDir + 1) : InvalidOid,
-						 manifest, spcoid);
+						 isDbDir ? atooid(lastDir + 1) : InvalidOid);
 		}
 		else
 			ereport(WARNING,
@@ -1318,7 +1308,7 @@ is_checksummed_file(const char *fullpath, const char *filename)
 static void
 archive_file(bbarchiver *archiver, const char *readfilename,
 			 const char *tarfilename, struct stat *statbuf, bool missing_ok,
-			 Oid dboid, backup_manifest_info *manifest, const char *spcoid)
+			 Oid dboid)
 {
 	int			fd;
 	BlockNumber blkno = 0;
@@ -1334,10 +1324,6 @@ archive_file(bbarchiver *archiver, const char *readfilename,
 	int			segmentno = 0;
 	char	   *segmentpath;
 	bool		verify_checksum = false;
-	pg_checksum_context checksum_ctx;
-
-	if (manifest != NULL)
-		pg_checksum_init(&checksum_ctx, manifest->checksum_type);
 
 	bbarchiver_begin_file(archiver, tarfilename, statbuf);
 
@@ -1512,10 +1498,6 @@ archive_file(bbarchiver *archiver, const char *readfilename,
 
 		bbarchiver_file_contents(archiver, buf, cnt);
 
-		/* Also feed it to the checksum machinery. */
-		if (manifest != NULL)
-			pg_checksum_update(&checksum_ctx, (uint8 *) buf, cnt);
-
 		len += cnt;
 	}
 
@@ -1527,8 +1509,6 @@ archive_file(bbarchiver *archiver, const char *readfilename,
 		{
 			cnt = Min(sizeof(buf), statbuf->st_size - len);
 			bbarchiver_file_contents(archiver, buf, cnt);
-			if (manifest != NULL)
-				pg_checksum_update(&checksum_ctx, (uint8 *) buf, cnt);
 			len += cnt;
 		}
 	}
@@ -1549,11 +1529,6 @@ archive_file(bbarchiver *archiver, const char *readfilename,
 	}
 
 	total_checksum_failures += checksum_failures;
-
-	if (manifest != NULL)
-		AddFileToBackupManifest(manifest, spcoid, tarfilename,
-								statbuf->st_size,
-								(pg_time_t) statbuf->st_mtime, &checksum_ctx);
 }
 
 /*
diff --git a/src/include/replication/backup_manifest.h b/src/include/replication/backup_manifest.h
index bbd08f1852..d49b890743 100644
--- a/src/include/replication/backup_manifest.h
+++ b/src/include/replication/backup_manifest.h
@@ -14,7 +14,7 @@
 
 #include "common/checksum_helper.h"
 #include "pgtime.h"
-#include "replication/basebackup_sink.h"
+#include "replication/basebackup_archiver.h"
 #include "storage/buffile.h"
 
 typedef enum manifest_option
@@ -49,4 +49,7 @@ extern void AddWALInfoToBackupManifest(backup_manifest_info *manifest,
 									   TimeLineID endtli);
 extern void SendBackupManifest(backup_manifest_info *manifest, bbsink *sink);
 
+extern bbarchiver *bbarchiver_manifest_new(bbarchiver *next,
+										   backup_manifest_info *manifest);
+
 #endif
-- 
2.24.3 (Apple Git-128)