From: Sergei Kornilov <sk@zsrv.org>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: a.lubennikova@postgrespro.ru <a.lubennikova@postgrespro.ru>
Cc: robertmhaas@gmail.com <robertmhaas@gmail.com>
Cc: michael@paquier.xyz <michael@paquier.xyz>
Cc: andres@anarazel.de <andres@anarazel.de>
Cc: peter.eisentraut@2ndquadrant.com <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: Allow some recovery parameters to be changed with reload
Date: Fri, 06 Nov 2020 15:36:48 +0300
Message-ID: <56311604665173@mail.yandex.ru> (raw)
In-Reply-To: <0e0d5982-ac64-818d-1ec6-18a80b9300b1@oss.nttdata.com>
References: <20200809052149.GB17986@paquier.xyz>
<CA+TgmobB4Syv3rh_v_QoyS=hMpw_Hf4JmjeOFQvDEQA8f2TFGw@mail.gmail.com>
<1741c40b-73e3-133b-77e6-05ebc7bf3c0f@postgrespro.ru>
<20201022.110026.524650499365127827.horikyota.ntt@gmail.com>
<4920201603879284@mail.yandex.ru>
<0e0d5982-ac64-818d-1ec6-18a80b9300b1@oss.nttdata.com>
Hello
> Currently when restore_command is not set, archive recovery fails
> at the beginning. With the patch, how should we treat the case where
> retore_command is reset to empty during archive recovery? We should
> reject that change of restore_command?
Good point. I think we should reject that change. But (AFAIC) I cannot use GUC check callback for this purpose, as only the startup process knows StandbyModeRequested. I think it would be appropriate to call validateRecoveryParameters from StartupRereadConfig. As side effect this add warning/hint "specified neither primary_conninfo nor restore_command" in standby mode in appropriate configuration state. Not sure about the rest checks in validateRecoveryParameters, maybe it's a wrong idea to recheck them here and I need to separate these checks into another function.
regards, Sergei
Attachments:v2_restore_command_reload.patch (text/x-diff, 3.3K) downloaddiff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgmlindex f043433e31..ec74cb43ad 100644--- a/doc/src/sgml/config.sgml+++ b/doc/src/sgml/config.sgml@@ -3542,7 +3542,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</para>
<para>
- This parameter can only be set at server start.+ This parameter can only be set in the <filename>postgresql.conf</filename>+ file or on the server command line.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.cindex a1078a7cfc..148dd34633 100644--- a/src/backend/access/transam/xlog.c+++ b/src/backend/access/transam/xlog.c@@ -883,7 +883,6 @@ static MemoryContext walDebugCxt = NULL;
#endif
static void readRecoverySignalFile(void);
-static void validateRecoveryParameters(void);
static void exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog);
static bool recoveryStopsBefore(XLogReaderState *record);
static bool recoveryStopsAfter(XLogReaderState *record);
@@ -5458,7 +5457,7 @@ readRecoverySignalFile(void)
errmsg("standby mode is not supported by single-user servers")));
}
-static void+void
validateRecoveryParameters(void)
{
if (!ArchiveRecoveryRequested)
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.cindex eab9c8c4ed..bf7dc866db 100644--- a/src/backend/postmaster/startup.c+++ b/src/backend/postmaster/startup.c@@ -114,6 +114,11 @@ StartupRereadConfig(void)
if (conninfoChanged || slotnameChanged || tempSlotChanged)
StartupRequestWalReceiverRestart();
++ /*+ * Check the combination of new parameters+ */+ validateRecoveryParameters();
}
/* Handle various signals that might be sent to the startup process */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.cindex a62d64eaa4..87fd593924 100644--- a/src/backend/utils/misc/guc.c+++ b/src/backend/utils/misc/guc.c@@ -3699,7 +3699,7 @@ static struct config_string ConfigureNamesString[] =
},
{
- {"restore_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,+ {"restore_command", PGC_SIGHUP, WAL_ARCHIVE_RECOVERY,
gettext_noop("Sets the shell command that will be called to retrieve an archived WAL file."),
NULL
},
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sampleindex 9cb571f7cc..9c9091e601 100644--- a/src/backend/utils/misc/postgresql.conf.sample+++ b/src/backend/utils/misc/postgresql.conf.sample@@ -253,7 +253,6 @@
# placeholders: %p = path of file to restore
# %f = file name only
# e.g. 'cp /mnt/server/archivedir/%f %p'
- # (change requires restart)
#archive_cleanup_command = '' # command to execute at every restartpoint
#recovery_end_command = '' # command to execute at completion of recovery
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.hindex 221af87e71..965ed109b3 100644--- a/src/include/access/xlog.h+++ b/src/include/access/xlog.h@@ -347,6 +347,7 @@ extern void WakeupRecovery(void);
extern void SetWalWriterSleeping(bool sleeping);
extern void StartupRequestWalReceiverRestart(void);
+extern void validateRecoveryParameters(void);
extern void XLogRequestWalReceiverReply(void);
extern void assign_max_wal_size(int newval, void *extra);
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: sk@zsrv.org, masao.fujii@oss.nttdata.com, horikyota.ntt@gmail.com, a.lubennikova@postgrespro.ru, robertmhaas@gmail.com, michael@paquier.xyz, andres@anarazel.de, peter.eisentraut@2ndquadrant.com
Subject: Re: Allow some recovery parameters to be changed with reload
In-Reply-To: <56311604665173@mail.yandex.ru>
* 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