Re: Allow some recovery parameters to be changed with reload

At Sat, 07 Nov 2020 00:36:33 +0300, Sergei Kornilov <sk@zsrv.org> wrote in 
> Hello
> 
> > I'm wondering if it's safe to allow restore_command to be emptied during
> > archive recovery. Even when it's emptied, archive recovery can proceed
> > by reading WAL files from pg_wal directory. This is the same behavior as
> > when restore_command is set to, e.g., /bin/false.
> 
> I am always confused by this implementation detail. restore_command fails? Fine, let's just read file from pg_wal. But this is different topic...

--- 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();

If someone changes restore_command to '' then reload while crash
recovery is running, the server stops for no valid reason. If
restore_command is set to 'hoge' (literally:p, that is, anything
unexecutable) and send SIGHUP while archive recovery is running, the
server stops.  I think we need to handle these cases more gracefully,
I think. That said, I think we should keep the current behavior that
the server stops if the same happens just after server start.

If someone changes restore_command by mistake to something executable
but fails to offer the specfied file even if it exists, the running
archive recovery finishes then switches timeline unexpectedly.  With
the same reasoning to the discussion abou inexecutable contents just
above, that behavior seems valid when the variable has not changed
since startup, but I'm not sure what to do if that happens by a reload
while (archive|crash) recovery is proceeding.

> I do not know the history of this fatal ereport. It looks like "must specify restore_command when standby mode is not enabled" check is only intended to protect the user from misconfiguration and the rest code will treat empty restore_command correctly, just like /bin/false. Did not notice anything around StandbyMode conditions.

If restore_command is not changable after server-start, it would be
valid for startup to stop for inexecutable content for the variable
since there's no way to proceed recovery.


regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center