From bb3896b115a964960a8b87063a1ac4ac5d7596bf Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Wed, 2 Feb 2022 09:50:34 -0500
Subject: [PATCH 1/2] Allow extensions to add new backup targets.

Commit 3500ccc39b0dadd1068a03938e4b8ff562587ccc allowed for base backup
targets, meaning that we could do something with the backup other than
send it to the client, but all of those targets had to be baked in to
the core code. This commit makes it possible for extensions to define
additional backup targets.
---
 src/backend/replication/Makefile            |   1 +
 src/backend/replication/basebackup.c        |  93 +++-----
 src/backend/replication/basebackup_target.c | 238 ++++++++++++++++++++
 src/include/replication/basebackup_target.h |  66 ++++++
 4 files changed, 340 insertions(+), 58 deletions(-)
 create mode 100644 src/backend/replication/basebackup_target.c
 create mode 100644 src/include/replication/basebackup_target.h

diff --git a/src/backend/replication/Makefile b/src/backend/replication/Makefile
index 8ec60ded76..847c87214f 100644
--- a/src/backend/replication/Makefile
+++ b/src/backend/replication/Makefile
@@ -22,6 +22,7 @@ OBJS = \
 	basebackup_progress.o \
 	basebackup_server.o \
 	basebackup_sink.o \
+	basebackup_target.o \
 	basebackup_throttle.o \
 	repl_gram.o \
 	slot.o \
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 10ce2406c0..7d4af84cf1 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -28,6 +28,7 @@
 #include "postmaster/syslogger.h"
 #include "replication/basebackup.h"
 #include "replication/basebackup_sink.h"
+#include "replication/basebackup_target.h"
 #include "replication/backup_manifest.h"
 #include "replication/walsender.h"
 #include "replication/walsender_private.h"
@@ -53,14 +54,6 @@
  */
 #define SINK_BUFFER_LENGTH			Max(32768, BLCKSZ)
 
-typedef enum
-{
-	BACKUP_TARGET_BLACKHOLE,
-	BACKUP_TARGET_COMPAT,
-	BACKUP_TARGET_CLIENT,
-	BACKUP_TARGET_SERVER
-} backup_target_type;
-
 typedef enum
 {
 	BACKUP_COMPRESSION_NONE,
@@ -76,8 +69,9 @@ typedef struct
 	bool		includewal;
 	uint32		maxrate;
 	bool		sendtblspcmapfile;
-	backup_target_type target;
-	char	   *target_detail;
+	bool		send_to_client;
+	bool		use_copytblspc;
+	BaseBackupTargetHandle *target_handle;
 	backup_manifest_option manifest;
 	basebackup_compression_type	compression;
 	int			compression_level;
@@ -714,12 +708,12 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 	bool		o_manifest_checksums = false;
 	bool		o_target = false;
 	bool		o_target_detail = false;
-	char	   *target_str = "compat";	/* placate compiler */
+	char	   *target_str = NULL;
+	char	   *target_detail_str = NULL;
 	bool		o_compression = false;
 	bool		o_compression_level = false;
 
 	MemSet(opt, 0, sizeof(*opt));
-	opt->target = BACKUP_TARGET_COMPAT;
 	opt->manifest = MANIFEST_OPTION_NO;
 	opt->manifest_checksum_type = CHECKSUM_TYPE_CRC32C;
 	opt->compression = BACKUP_COMPRESSION_NONE;
@@ -863,22 +857,11 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 		}
 		else if (strcmp(defel->defname, "target") == 0)
 		{
-			target_str = defGetString(defel);
-
 			if (o_target)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			if (strcmp(target_str, "blackhole") == 0)
-				opt->target = BACKUP_TARGET_BLACKHOLE;
-			else if (strcmp(target_str, "client") == 0)
-				opt->target = BACKUP_TARGET_CLIENT;
-			else if (strcmp(target_str, "server") == 0)
-				opt->target = BACKUP_TARGET_SERVER;
-			else
-				ereport(ERROR,
-						(errcode(ERRCODE_SYNTAX_ERROR),
-						 errmsg("unrecognized target: \"%s\"", target_str)));
+			target_str = defGetString(defel);
 			o_target = true;
 		}
 		else if (strcmp(defel->defname, "target_detail") == 0)
@@ -889,7 +872,7 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			opt->target_detail = optval;
+			target_detail_str = optval;
 			o_target_detail = true;
 		}
 		else if (strcmp(defel->defname, "compression") == 0)
@@ -937,22 +920,28 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 					 errmsg("manifest checksums require a backup manifest")));
 		opt->manifest_checksum_type = CHECKSUM_TYPE_NONE;
 	}
-	if (opt->target == BACKUP_TARGET_SERVER)
+
+	if (target_str == NULL)
 	{
-		if (opt->target_detail == NULL)
+		if (target_detail_str != NULL)
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
-					 errmsg("target '%s' requires a target detail",
-							target_str)));
+					 errmsg("target detail cannot be used without target")));
+		opt->use_copytblspc = true;
+		opt->send_to_client = true;
 	}
-	else
+	else if (strcmp(target_str, "client") == 0)
 	{
-		if (opt->target_detail != NULL)
+		if (target_detail_str != NULL)
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
 					 errmsg("target '%s' does not accept a target detail",
 							target_str)));
+		opt->send_to_client = true;
 	}
+	else
+		opt->target_handle =
+			BaseBackupGetTargetHandle(target_str, target_detail_str);
 
 	if (o_compression_level && !o_compression)
 		ereport(ERROR,
@@ -988,37 +977,25 @@ SendBaseBackup(BaseBackupCmd *cmd)
 	}
 
 	/*
-	 * If the TARGET option was specified, then we can use the new copy-stream
-	 * protocol. If the target is specifically 'client' then set up to stream
-	 * the backup to the client; otherwise, it's being sent someplace else and
-	 * should not be sent to the client.
+	 * There are basically three cases here.
+	 *
+	 * If the TARGET option was not specified, we have to fall back to the
+	 * older and less capable copy-tablespace protocol.
+	 *
+	 * If the TARGET option was set to 'client', we'll use the new copy-stream
+	 * protocol and send the backup to the client.
 	 *
-	 * If the TARGET option was not specified, we must fall back to the older
-	 * and less capable copy-tablespace protocol.
+	 * If the TARGET option was set to anything else, we'll use the new
+	 * copy-stream protocol to send progress updates to the client, and
+	 * BaseBackupGetSink will arrange to dispose of the backup data.
 	 */
-	if (opt.target == BACKUP_TARGET_CLIENT)
-		sink = bbsink_copystream_new(true);
-	else if (opt.target != BACKUP_TARGET_COMPAT)
-		sink = bbsink_copystream_new(false);
-	else
+	if (opt.use_copytblspc)
 		sink = bbsink_copytblspc_new();
-
-	/*
-	 * If a non-default backup target is in use, arrange to send the data
-	 * wherever it needs to go.
-	 */
-	switch (opt.target)
+	else
 	{
-		case BACKUP_TARGET_BLACKHOLE:
-			/* Nothing to do, just discard data. */
-			break;
-		case BACKUP_TARGET_COMPAT:
-		case BACKUP_TARGET_CLIENT:
-			/* Nothing to do, handling above is sufficient. */
-			break;
-		case BACKUP_TARGET_SERVER:
-			sink = bbsink_server_new(sink, opt.target_detail);
-			break;
+		sink = bbsink_copystream_new(opt.send_to_client);
+		if (opt.target_handle != NULL)
+			sink = BaseBackupGetSink(opt.target_handle, sink);
 	}
 
 	/* Set up network throttling, if client requested it */
diff --git a/src/backend/replication/basebackup_target.c b/src/backend/replication/basebackup_target.c
new file mode 100644
index 0000000000..d93f5e02db
--- /dev/null
+++ b/src/backend/replication/basebackup_target.c
@@ -0,0 +1,238 @@
+/*-------------------------------------------------------------------------
+ *
+ * basebackup_target.c
+ *	  Base backups can be "targetted," which means that they can be sent
+ *	  somewhere other than to the client which requested the backup.
+ *	  Furthermore, new targets can be defined by extensions. This file
+ *	  contains code to support that functionality.
+ *
+ * Portions Copyright (c) 2010-2020, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *	  src/backend/replication/basebackup_gzip.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "replication/basebackup_target.h"
+#include "utils/memutils.h"
+
+typedef struct BaseBackupTargetType
+{
+	char	   *name;
+	void	   *(*check_detail) (char *, char *);
+	bbsink	   *(*get_sink) (bbsink *, void *);
+} BaseBackupTargetType;
+
+struct BaseBackupTargetHandle
+{
+	BaseBackupTargetType *type;
+	void	   *detail_arg;
+};
+
+static void initialize_target_list(void);
+extern bbsink *blackhole_get_sink(bbsink *next_sink, void *detail_arg);
+extern bbsink *server_get_sink(bbsink *next_sink, void *detail_arg);
+static void *reject_target_detail(char *target, char *target_detail);
+static void *server_check_detail(char *target, char *target_detail);
+
+static BaseBackupTargetType builtin_backup_targets[] =
+{
+	{
+		"blackhole", reject_target_detail, blackhole_get_sink
+	},
+	{
+		"server", server_check_detail, server_get_sink
+	},
+	{
+		NULL
+	}
+};
+
+static List *BaseBackupTargetTypeList = NIL;
+
+/*
+ * Add a new base backup target type.
+ *
+ * This is intended for use by server extensions.
+ */
+void
+BaseBackupAddTarget(char *name,
+					void *(*check_detail) (char *, char *),
+					bbsink *(*get_sink) (bbsink *, void *))
+{
+	BaseBackupTargetType *ttype;
+	MemoryContext oldcontext;
+	ListCell   *lc;
+
+	/* If the target list is not yet initialized, do that first. */
+	if (BaseBackupTargetTypeList == NIL)
+		initialize_target_list();
+
+	/* Search the target type list for an existing entry with this name. */
+	foreach(lc, BaseBackupTargetTypeList)
+	{
+		BaseBackupTargetType *ttype = lfirst(lc);
+
+		if (strcmp(ttype->name, name) == 0)
+		{
+			/*
+			 * We found one, so update it.
+			 *
+			 * It is probably not a great idea to call BaseBackupAddTarget
+			 * for the same name multiple times, but if it happens, this
+			 * seems like the sanest behavior.
+			 */
+			ttype->check_detail = check_detail;
+			ttype->get_sink = get_sink;
+			return;
+		}
+	}
+
+	/*
+	 * We use TopMemoryContext for allocations here to make sure that the
+	 * data we need doesn't vanish under us; that's also why we copy the
+	 * target name into a newly-allocated chunk of memory.
+	 */
+	oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+	ttype = palloc(sizeof(BaseBackupTargetType));
+	ttype->name = pstrdup(name);
+	ttype->check_detail = check_detail;
+	ttype->get_sink = get_sink;
+	BaseBackupTargetTypeList = lappend(BaseBackupTargetTypeList, ttype);
+	MemoryContextSwitchTo(oldcontext);
+}
+
+/*
+ * Look up a base backup target and validate the target_detail.
+ *
+ * Extensions that define new backup targets will probably define a new
+ * type of bbsink to match. Validation of the target_detail can be performed
+ * either in the check_detail routine called here, or in the bbsink
+ * constructor, which will be called from BaseBackupGetSink. It's mostly
+ * a matter of taste, but the check_detail function runs somewhat earlier.
+ */
+BaseBackupTargetHandle *
+BaseBackupGetTargetHandle(char *target, char *target_detail)
+{
+	ListCell   *lc;
+
+	/* If the target list is not yet initialized, do that first. */
+	if (BaseBackupTargetTypeList == NIL)
+		initialize_target_list();
+
+	/* Search the target type list for a match. */
+	foreach(lc, BaseBackupTargetTypeList)
+	{
+		BaseBackupTargetType *ttype = lfirst(lc);
+
+		if (strcmp(ttype->name, target) == 0)
+		{
+			BaseBackupTargetHandle *handle;
+
+			/* Found the target. */
+			handle = palloc(sizeof(BaseBackupTargetHandle));
+			handle->type = ttype;
+			handle->detail_arg = ttype->check_detail(target, target_detail);
+
+			return handle;
+		}
+	}
+
+	/* Did not find the target. */
+	ereport(ERROR,
+			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+			 errmsg("unrecognized target: \"%s\"", target)));
+}
+
+/*
+ * Construct a bbsink that will implement the backup target.
+ *
+ * The get_sink function does all the real work, so all we have to do here
+ * is call it with the correct arguments. Whatever the check_detail function
+ * returned is here passed through to the get_sink function. This lets those
+ * two functions communicate with each other, if they wish. If not, the
+ * check_detail function can simply return the target_detail and let the
+ * get_sink function take it from there.
+ */
+bbsink *
+BaseBackupGetSink(BaseBackupTargetHandle *handle, bbsink *next_sink)
+{
+	return handle->type->get_sink(next_sink, handle->detail_arg);
+}
+
+/*
+ * Load predefined target types into BaseBackupTargetTypeList.
+ */
+static void
+initialize_target_list(void)
+{
+	BaseBackupTargetType *ttype = builtin_backup_targets;
+	MemoryContext oldcontext;
+
+	oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+	while (ttype->name != NULL)
+	{
+		BaseBackupTargetTypeList = lappend(BaseBackupTargetTypeList, ttype);
+		++ttype;
+	}
+	MemoryContextSwitchTo(oldcontext);
+}
+
+/*
+ * Normally, a get_sink function should construct and return a new bbsink that
+ * implements the backup target, but the 'blackhole' target just throws the
+ * data away. We could implement that by adding a bbsink that does nothing
+ * but forward, but it's even cheaper to implement that by not adding a bbsink
+ * at all.
+ */
+bbsink *
+blackhole_get_sink(bbsink *next_sink, void *detail_arg)
+{
+	return next_sink;
+}
+
+/*
+ * Create a bbsink implementing a server-side backup.
+ */
+bbsink *
+server_get_sink(bbsink *next_sink, void *detail_arg)
+{
+	return bbsink_server_new(next_sink, detail_arg);
+}
+
+/*
+ * Implement target-detail checking for a target that does not accept a
+ * detail.
+ */
+void *
+reject_target_detail(char *target, char *target_detail)
+{
+	if (target_detail != NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("target '%s' does not accept a target detail",
+						target)));
+
+	return NULL;
+}
+
+/*
+ * Implement target-detail checking for a server-side backup.
+ *
+ * target_detail should be the name of the directory to which the backup
+ * should be written, but we don't check that here. Rather, that check,
+ * as well as the necessary permissions checking, happens in bbsink_server_new.
+ */
+void *
+server_check_detail(char *target, char *target_detail)
+{
+	if (target_detail == NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("target '%s' requires a target detail",
+						target)));
+
+	return target_detail;
+}
diff --git a/src/include/replication/basebackup_target.h b/src/include/replication/basebackup_target.h
new file mode 100644
index 0000000000..e23ac29a89
--- /dev/null
+++ b/src/include/replication/basebackup_target.h
@@ -0,0 +1,66 @@
+/*-------------------------------------------------------------------------
+ *
+ * basebackup_target.h
+ *	  Extensibility framework for adding base backup targets.
+ *
+ * Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
+ *
+ * src/include/replication/basebackup_target.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef BASEBACKUP_TARGET_H
+#define BASEBACKUP_TARGET_H
+
+#include "replication/basebackup_sink.h"
+
+struct BaseBackupTargetHandle;
+typedef struct BaseBackupTargetHandle BaseBackupTargetHandle;
+
+/*
+ * Extensions can call this function to create new backup targets.
+ *
+ * 'name' is the name of the new target.
+ *
+ * 'check_detail' is a function that accepts a target name and target detail
+ * and either throws an error (if the target detail is not valid or some other
+ * problem, such as a permissions issue, is detected) or returns a pointer to
+ * the data that will be needed to create a bbsink implementing that target.
+ * The second argumnt will be NULL if the TARGET_DETAIL option to the
+ * BASE_BACKUP command was not specified.
+ *
+ * 'get_sink' is a function that creates the bbsink. The first argument
+ * is the successor sink; the sink created by this function should always
+ * forward to this sink. The second argument is the pointer returned by a
+ * previous call to the 'check_detail' function.
+ *
+ * In practice, a user will type something like "pg_basebackup --target foo:bar
+ * -Xfetch". That will cause the server to look for a backup target named
+ * "foo". If one is found, the check_detail callback will be invoked for the
+ * string "bar", and whatever that callback returns will be passed as the
+ * second argument to the get_sink callback.
+ */
+extern void BaseBackupAddTarget(char *name,
+								void *(*check_detail) (char *, char *),
+								bbsink * (*get_sink) (bbsink *, void *));
+
+/*
+ * These functions are used by the core code to access base backup targets
+ * added via BaseBackupAddTarget(). The core code will pass the TARGET and
+ * TARGET_DETAIL strings obtained from the user to BaseBackupGetTargetHandle,
+ * which will either throw an error (if the TARGET is not recognized or the
+ * check_detail hook for that TARGET doesn't like the TARGET_DETAIL) or
+ * return a BaseBackupTargetHandle object that can later be passed to
+ * BaseBackupGetSink.
+ *
+ * BaseBackupGetSink constructs a bbsink implementing the desired target
+ * using the BaseBackupTargetHandle and the successor bbsink. It does this
+ * by arranging to call the get_sink() callback provided by the extension
+ * that implements the base backup target.
+ */
+extern BaseBackupTargetHandle *BaseBackupGetTargetHandle(char *target,
+														 char *target_detail);
+extern bbsink *BaseBackupGetSink(BaseBackupTargetHandle *handle,
+								 bbsink *next_sink);
+
+#endif
-- 
2.24.3 (Apple Git-128)

