Re: Allow some recovery parameters to be changed with reload

Sergei Kornilov <sk@zsrv.org>

From: Sergei Kornilov <sk@zsrv.org>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: "masao.fujii@oss.nttdata.com" <masao.fujii@oss.nttdata.com>, "a.lubennikova@postgrespro.ru" <a.lubennikova@postgrespro.ru>, "robertmhaas@gmail.com" <robertmhaas@gmail.com>, "michael@paquier.xyz" <michael@paquier.xyz>, "andres@anarazel.de" <andres@anarazel.de>, "peter.eisentraut@2ndquadrant.com" <peter.eisentraut@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-11-10T11:13:17Z
Lists: pgsql-hackers
Hello

> If someone changes restore_command to '' then reload while crash
> recovery is running, the server stops for no valid reason.

While *crash* recovery is running? It's possible only during Point-in-Time Recovery, no?
At the beginning of validateRecoveryParameters we check ArchiveRecoveryRequested, which can be set in two cases:

* if recovery.signal found - same check on recovery start. Otherwise it is possible to early end recovery because of empty restore_command. So we want to protect the user from such misconfiguration? I am fine if we decide that no additional handling is needed.
* if standby.signal found - this FATAL is not reachable because StandbyModeRequested is also set.

During crash recovery validateRecoveryParameters does nothing.

> 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 we can not perform such check reliable. As in my example earlier:

> restore_command = '. /etc/wal-g/WALG_AWS_ENV; wal-g wal-fetch "%f" "%p"'

How do we find the commands first? For any shell? And even: we learned that the binary is unexecutable. But what to do next?

> 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.

Or executable file was just removed. Which is clearly a pilot error. Is this differs from changing restore_command?

>>  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.

Why not use local pg_wal? There may be already enough WAL.

regards, Sergei



Commits

  1. Allow restore_command parameter to be changed with reload.

  2. Stamp 13.0.

  3. Allow some recovery parameters to be changed with reload