pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: Benoit Lobréau <benoit.lobreau@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Bossart, Nathan <bossartn@amazon.com>
Cc: David Steele <david@pgmasters.net>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: Stephen Frost <sfrost@snowman.net>
Subject: Re: archive modules
Date: Fri, 14 Oct 2022 14:42:56 -0700
Message-ID: <20221014214256.GA1819154@nathanxps13> (raw)
In-Reply-To: <20221014185130.GC1678424@nathanxps13>
References: <20220923161402.GA736566@nathanxps13>
	<4f66de67-35a0-3fe7-fa61-d618e1bd2c2c@enterprisedb.com>
	<20221005185716.GB201192@nathanxps13>
	<1d9cb989-91b0-919e-f42b-4788f5fc7911@enterprisedb.com>
	<CALj2ACWAq_gTQ-FhjFvGjhOd5+7qWdepVmYcqsZ_whENshg1zw@mail.gmail.com>
	<20221013182408.GA1551106@nathanxps13>
	<1061703.1665687218@sss.pgh.pa.us>
	<Y0itpip6v7rYmAJM@paquier.xyz>
	<CALj2ACXYQckdBWQTpJDCRd9r7xxJsdnbuHp-DnNEAWpiPZwtfA@mail.gmail.com>
	<20221014185130.GC1678424@nathanxps13>

On Fri, Oct 14, 2022 at 11:51:30AM -0700, Nathan Bossart wrote:
> On Fri, Oct 14, 2022 at 12:10:18PM +0530, Bharath Rupireddy wrote:
>> 2) I think we have a problem - set archive_mode and archive_library
>> and start the server, then set archive_command, reload the conf, see
>> [3] - the archiver needs to error out right? The archiver gets
>> restarted whenever archive_library changes but not when
>> archive_command changes. I think the right place for the error is
>> after or at the end of HandlePgArchInterrupts().
> 
> Good catch.  You are right, this is broken.  I believe that we need to
> check for the misconfiguration in HandlePgArchInterrupts() in addition to
> LoadArchiveLibrary().  I will work on fixing this.

As promised...

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachments:

  [text/x-diff] fail_arch_v2.patch (4.0K, ../20221014214256.GA1819154@nathanxps13/2-fail_arch_v2.patch)
  download | inline diff:
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 66312b53b8..9d0f3608c4 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3597,9 +3597,11 @@ include_dir 'conf.d'
        </para>
        <para>
         This parameter can only be set in the <filename>postgresql.conf</filename>
-        file or on the server command line.  It is ignored unless
+        file or on the server command line.  It is only used if
         <varname>archive_mode</varname> was enabled at server start and
-        <varname>archive_library</varname> is set to an empty string.
+        <varname>archive_library</varname> is set to an empty string.  If both
+        <varname>archive_command</varname> and <varname>archive_library</varname>
+        are set, archiving will fail.
         If <varname>archive_command</varname> is an empty string (the default) while
         <varname>archive_mode</varname> is enabled (and <varname>archive_library</varname>
         is set to an empty string), WAL archiving is temporarily
@@ -3624,7 +3626,9 @@ include_dir 'conf.d'
        <para>
         The library to use for archiving completed WAL file segments.  If set to
         an empty string (the default), archiving via shell is enabled, and
-        <xref linkend="guc-archive-command"/> is used.  Otherwise, the specified
+        <xref linkend="guc-archive-command"/> is used.  If both
+        <varname>archive_command</varname> and <varname>archive_library</varname>
+        are set, archiving will fail.  Otherwise, the specified
         shared library is used for archiving.  For more information, see
         <xref linkend="backup-archiving-wal"/> and
         <xref linkend="archive-modules"/>.
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 3868cd7bd3..39c2115943 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -801,7 +801,8 @@ HandlePgArchInterrupts(void)
 		archiveLibChanged = strcmp(XLogArchiveLibrary, archiveLib) != 0;
 		pfree(archiveLib);
 
-		if (archiveLibChanged)
+		if (archiveLibChanged ||
+			(XLogArchiveLibrary[0] != '\0' && XLogArchiveCommand[0] != '\0'))
 		{
 			/*
 			 * Call the currently loaded archive module's shutdown callback,
@@ -809,17 +810,25 @@ HandlePgArchInterrupts(void)
 			 */
 			call_archive_module_shutdown_callback(0, 0);
 
-			/*
-			 * Ideally, we would simply unload the previous archive module and
-			 * load the new one, but there is presently no mechanism for
-			 * unloading a library (see the comment above
-			 * internal_load_library()).  To deal with this, we simply restart
-			 * the archiver.  The new archive module will be loaded when the
-			 * new archiver process starts up.
-			 */
-			ereport(LOG,
-					(errmsg("restarting archiver process because value of "
-							"\"archive_library\" was changed")));
+			if (archiveLibChanged)
+			{
+				/*
+				 * Ideally, we would simply unload the previous archive module
+				 * and load the new one, but there is presently no mechanism
+				 * for unloading a library (see the comment above
+				 * internal_load_library()).  To deal with this, we simply
+				 * restart the archiver.  The new archive module will be loaded
+				 * when the new archiver process starts up.
+				 */
+				ereport(LOG,
+						(errmsg("restarting archiver process because value of "
+								"\"archive_library\" was changed")));
+			}
+			else
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("both archive_command and archive_library specified"),
+						 errdetail("Only one of archive_command, archive_library may be set.")));
 
 			proc_exit(0);
 		}
@@ -836,6 +845,12 @@ LoadArchiveLibrary(void)
 {
 	ArchiveModuleInit archive_init;
 
+	if (XLogArchiveLibrary[0] != '\0' && XLogArchiveCommand[0] != '\0')
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("both archive_command and archive_library specified"),
+				 errdetail("Only one of archive_command, archive_library may be set.")));
+
 	memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks));
 
 	/*

view thread (102+ messages)  latest in thread

Message-ID: <20221014214256.GA1819154@nathanxps13>
Permalink:  ../20221014214256.GA1819154@nathanxps13/
Also on:    postgresql.org/message-id/20221014214256.GA1819154@nathanxps13

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, bharath.rupireddyforpostgres@gmail.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, peter.eisentraut@enterprisedb.com, benoit.lobreau@gmail.com, alvherre@alvh.no-ip.org, robertmhaas@gmail.com, bossartn@amazon.com, david@pgmasters.net, masao.fujii@oss.nttdata.com, magnus@hagander.net, sfrost@snowman.net
  Subject: Re: archive modules
  In-Reply-To: <20221014214256.GA1819154@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox