Redesign archive modules

A new callback named startup_cb, called shortly after a module is
loaded, is added.  This makes possible the initialization of any
additional state data required by a module.  This initial state data can
be saved in a ArchiveModuleState, that is now passed down to all the
callbacks that can be defined in a module.  With this design, it is
possible to have a per-module state, aimed at opening the door to the
support of more than one archive module.

The initialization of the callbacks is changed so as
_PG_archive_module_init() does not anymore give in input a
ArchiveModuleCallbacks that a module has to fill in with callback
definitions.  Instead, a module now needs to return a const
ArchiveModuleCallbacks.

All the structure and callback definitions of archive modules are moved
into their own header, named archive_module.h, from pgarch.h.
Command-based archiving follows the same line, with a new set of files
named shell_archive.{c,h}.

There are a few more items that are under discussion to improve the
design of archive modules, like the fact that basic_archive calls
sigsetjmp() by itself to define its own error handling flow.  These will
be adjusted later, the changes done here cover already a good portion
of what has been discussed.

Any modules created for v15 will need to be adjusted to this new
design.

Author: Nathan Bossart
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20230130194810.6fztfgbn32e7qarj@awork3.anarazel.de

4 files changed, 16 insertions, 142 deletions

diff --git a/src/backend/postmaster/Makefile b/src/backend/postmaster/Makefile
index 3a794e54d60..047448b34eb 100644
--- a/src/backend/postmaster/Makefile
+++ b/src/backend/postmaster/Makefile
@@ -22,7 +22,6 @@ OBJS = \
 	interrupt.o \
 	pgarch.o \
 	postmaster.o \
-	shell_archive.o \
 	startup.o \
 	syslogger.o \
 	walwriter.o
diff --git a/src/backend/postmaster/meson.build b/src/backend/postmaster/meson.build
index 9079922de7a..cda921fd10b 100644
--- a/src/backend/postmaster/meson.build
+++ b/src/backend/postmaster/meson.build
@@ -10,7 +10,6 @@ backend_sources += files(
   'interrupt.c',
   'pgarch.c',
   'postmaster.c',
-  'shell_archive.c',
   'startup.c',
   'syslogger.c',
   'walwriter.c',
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af29052..46af3495644 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -31,6 +31,8 @@
 
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
 #include "lib/binaryheap.h"
 #include "libpq/pqsignal.h"
 #include "pgstat.h"
@@ -97,7 +99,8 @@ char	   *XLogArchiveLibrary = "";
  */
 static time_t last_sigterm_time = 0;
 static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveContext;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
 
 
 /*
@@ -406,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
 			HandlePgArchInterrupts();
 
 			/* can't do anything if not configured ... */
-			if (ArchiveContext.check_configured_cb != NULL &&
-				!ArchiveContext.check_configured_cb())
+			if (ArchiveCallbacks->check_configured_cb != NULL &&
+				!ArchiveCallbacks->check_configured_cb(archive_module_state))
 			{
 				ereport(WARNING,
 						(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -508,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
 	snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
 	set_ps_display(activitymsg);
 
-	ret = ArchiveContext.archive_file_cb(xlog, pathname);
+	ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
 	if (ret)
 		snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
 	else
@@ -814,7 +817,7 @@ HandlePgArchInterrupts(void)
 /*
  * LoadArchiveLibrary
  *
- * Loads the archiving callbacks into our local ArchiveContext.
+ * Loads the archiving callbacks into our local ArchiveCallbacks.
  */
 static void
 LoadArchiveLibrary(void)
@@ -827,8 +830,6 @@ LoadArchiveLibrary(void)
 				 errmsg("both archive_command and archive_library set"),
 				 errdetail("Only one of archive_command, archive_library may be set.")));
 
-	memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks));
-
 	/*
 	 * If shell archiving is enabled, use our special initialization function.
 	 * Otherwise, load the library and call its _PG_archive_module_init().
@@ -844,12 +845,16 @@ LoadArchiveLibrary(void)
 		ereport(ERROR,
 				(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
 
-	(*archive_init) (&ArchiveContext);
+	ArchiveCallbacks = (*archive_init) ();
 
-	if (ArchiveContext.archive_file_cb == NULL)
+	if (ArchiveCallbacks->archive_file_cb == NULL)
 		ereport(ERROR,
 				(errmsg("archive modules must register an archive callback")));
 
+	archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+	if (ArchiveCallbacks->startup_cb != NULL)
+		ArchiveCallbacks->startup_cb(archive_module_state);
+
 	before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
 }
 
@@ -859,6 +864,6 @@ LoadArchiveLibrary(void)
 static void
 pgarch_call_module_shutdown_cb(int code, Datum arg)
 {
-	if (ArchiveContext.shutdown_cb != NULL)
-		ArchiveContext.shutdown_cb();
+	if (ArchiveCallbacks->shutdown_cb != NULL)
+		ArchiveCallbacks->shutdown_cb(archive_module_state);
 }
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
deleted file mode 100644
index 7771b951b7b..00000000000
--- a/src/backend/postmaster/shell_archive.c
+++ /dev/null
@@ -1,129 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * shell_archive.c
- *
- * This archiving function uses a user-specified shell command (the
- * archive_command GUC) to copy write-ahead log files.  It is used as the
- * default, but other modules may define their own custom archiving logic.
- *
- * Copyright (c) 2022-2023, PostgreSQL Global Development Group
- *
- * IDENTIFICATION
- *	  src/backend/postmaster/shell_archive.c
- *
- *-------------------------------------------------------------------------
- */
-#include "postgres.h"
-
-#include <sys/wait.h>
-
-#include "access/xlog.h"
-#include "common/percentrepl.h"
-#include "pgstat.h"
-#include "postmaster/pgarch.h"
-
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
-{
-	cb->check_configured_cb = shell_archive_configured;
-	cb->archive_file_cb = shell_archive_file;
-	cb->shutdown_cb = shell_archive_shutdown;
-}
-
-static bool
-shell_archive_configured(void)
-{
-	return XLogArchiveCommand[0] != '\0';
-}
-
-static bool
-shell_archive_file(const char *file, const char *path)
-{
-	char	   *xlogarchcmd;
-	char	   *nativePath = NULL;
-	int			rc;
-
-	if (path)
-	{
-		nativePath = pstrdup(path);
-		make_native_path(nativePath);
-	}
-
-	xlogarchcmd = replace_percent_placeholders(XLogArchiveCommand, "archive_command", "fp", file, nativePath);
-
-	if (nativePath)
-		pfree(nativePath);
-
-	ereport(DEBUG3,
-			(errmsg_internal("executing archive command \"%s\"",
-							 xlogarchcmd)));
-
-	fflush(NULL);
-	pgstat_report_wait_start(WAIT_EVENT_ARCHIVE_COMMAND);
-	rc = system(xlogarchcmd);
-	pgstat_report_wait_end();
-
-	if (rc != 0)
-	{
-		/*
-		 * If either the shell itself, or a called command, died on a signal,
-		 * abort the archiver.  We do this because system() ignores SIGINT and
-		 * SIGQUIT while waiting; so a signal is very likely something that
-		 * should have interrupted us too.  Also die if the shell got a hard
-		 * "command not found" type of error.  If we overreact it's no big
-		 * deal, the postmaster will just start the archiver again.
-		 */
-		int			lev = wait_result_is_any_signal(rc, true) ? FATAL : LOG;
-
-		if (WIFEXITED(rc))
-		{
-			ereport(lev,
-					(errmsg("archive command failed with exit code %d",
-							WEXITSTATUS(rc)),
-					 errdetail("The failed archive command was: %s",
-							   xlogarchcmd)));
-		}
-		else if (WIFSIGNALED(rc))
-		{
-#if defined(WIN32)
-			ereport(lev,
-					(errmsg("archive command was terminated by exception 0x%X",
-							WTERMSIG(rc)),
-					 errhint("See C include file \"ntstatus.h\" for a description of the hexadecimal value."),
-					 errdetail("The failed archive command was: %s",
-							   xlogarchcmd)));
-#else
-			ereport(lev,
-					(errmsg("archive command was terminated by signal %d: %s",
-							WTERMSIG(rc), pg_strsignal(WTERMSIG(rc))),
-					 errdetail("The failed archive command was: %s",
-							   xlogarchcmd)));
-#endif
-		}
-		else
-		{
-			ereport(lev,
-					(errmsg("archive command exited with unrecognized status %d",
-							rc),
-					 errdetail("The failed archive command was: %s",
-							   xlogarchcmd)));
-		}
-
-		return false;
-	}
-
-	pfree(xlogarchcmd);
-
-	elog(DEBUG1, "archived write-ahead log file \"%s\"", file);
-	return true;
-}
-
-static void
-shell_archive_shutdown(void)
-{
-	elog(DEBUG1, "archiver process shutting down");
-}