Allow extensions to add new backup targets.

Commit 3500ccc39b 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.

Patch by me, reviewed by Abhijit Menon-Sen.

Discussion: http://postgr.es/m/CA+TgmoaqvdT-u3nt+_kkZ7bgDAyqDB0i-+XOMmr5JN2Rd37hxw@mail.gmail.com

src/backend/replication/Makefile

@@ -24,6 +24,7 @@ OBJS = \
 	basebackup_progress.o \
 	basebackup_server.o \
 	basebackup_sink.o \
+	basebackup_target.o \
 	basebackup_throttle.o \
 	repl_gram.o \
 	slot.o \

src/backend/replication/Makefile.orig

@@ -0,0 +1,49 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+#    Makefile for src/backend/replication
+#
+# IDENTIFICATION
+#    src/backend/replication/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/replication
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
+
+OBJS = \
+	backup_manifest.o \
+	basebackup.o \
+	basebackup_copy.o \
+	basebackup_gzip.o \
+	basebackup_lz4.o \
+	basebackup_zstd.o \
+	basebackup_progress.o \
+	basebackup_server.o \
+	basebackup_sink.o \
+	basebackup_throttle.o \
+	repl_gram.o \
+	slot.o \
+	slotfuncs.o \
+	syncrep.o \
+	syncrep_gram.o \
+	walreceiver.o \
+	walreceiverfuncs.o \
+	walsender.o
+
+SUBDIRS = logical
+
+include $(top_srcdir)/src/backend/common.mk
+
+# repl_scanner is compiled as part of repl_gram
+repl_gram.o: repl_scanner.c
+
+# syncrep_scanner is compiled as part of syncrep_gram
+syncrep_gram.o: syncrep_scanner.c
+
+# repl_gram.c, repl_scanner.c, syncrep_gram.c and syncrep_scanner.c
+# are in the distribution tarball, so they are not cleaned here.
+# (Our parent Makefile takes care of them during maintainer-clean.)

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,13 +54,6 @@
  */
 #define SINK_BUFFER_LENGTH			Max(32768, BLCKSZ)
 
-typedef enum
-{
-	BACKUP_TARGET_BLACKHOLE,
-	BACKUP_TARGET_CLIENT,
-	BACKUP_TARGET_SERVER
-} backup_target_type;
-
 typedef enum
 {
 	BACKUP_COMPRESSION_NONE,
@@ -77,8 +71,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;
@@ -715,12 +710,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_CLIENT;
 	opt->manifest = MANIFEST_OPTION_NO;
 	opt->manifest_checksum_type = CHECKSUM_TYPE_CRC32C;
 	opt->compression = BACKUP_COMPRESSION_NONE;
@@ -864,22 +859,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)
@@ -890,7 +874,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)
@@ -942,22 +926,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,
@@ -993,32 +983,14 @@ 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.
+	 * 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. BaseBackupGetSink has the job of setting up a
+	 * sink to send the backup data wherever it needs to go.
 	 */
-	if (opt.target == BACKUP_TARGET_CLIENT)
-		sink = bbsink_copystream_new(true);
-	else
-		sink = bbsink_copystream_new(false);
-
-	/*
-	 * If a non-default backup target is in use, arrange to send the data
-	 * wherever it needs to go.
-	 */
-	switch (opt.target)
-	{
-		case BACKUP_TARGET_BLACKHOLE:
-			/* Nothing to do, just discard data. */
-			break;
-		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 */
 	if (opt.maxrate > 0)

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;
+}