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
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)); /*