Re: allow online change primary_conninfo

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Sergei Kornilov <sk@zsrv.org>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "michael@paquier.xyz" <michael@paquier.xyz>, "david@pgmasters.net" <david@pgmasters.net>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2019-09-20T18:09:48Z
Lists: pgsql-hackers
Hi,

On 2019-09-19 17:46:06 +0300, Sergei Kornilov wrote:

>           <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.
>            This setting has no effect if the server is not in standby mode.
>           </para>
> +         <para>
> +          If <varname>primary_conninfo</varname> is changed while WAL receiver is running,
> +          the WAL receiver shuts down and then restarts with new setting,
> +          except when primary_conninfo is an empty string.
> +         </para>

From the sentence structure it's not clear that "except when
primary_conninfo is an empty string" only applies to "and then restarts
with new setting".


> +/*
> + * Need for restart running WalReceiver due the configuration change.
> + * Suitable only for XLOG_FROM_STREAM source
> + */
> +static bool pendingWalRcvRestart = false;

s/due the/due to a/ (or even "due to the").


> @@ -11862,6 +11869,17 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
>  					if (WalRcvStreaming())
>  						ShutdownWalRcv();
>  
> +					/*
> +					 * If wal receiver is requested to restart, we skip the
> +					 * next XLOG_FROM_ARCHIVE to immediately starting it.
> +					 */
> +					if (pendingWalRcvRestart)
> +					{
> +						lastSourceFailed = true;
> +						currentSource = XLOG_FROM_ARCHIVE;
> +						continue;
> +					}

I can't parse that comment. What does "skipping to starting" mean? I
assume it's just about avoiding wal_retrieve_retry_interval, but I think
the comment ought to be rephrased.

Also, should we really check this before scanning for new timelines?

Why is it the right thing to change to XLOG_FROM_ARCHIVE when we're just
restarting walreceiver? The server might unnecessarily get stuck in
archive based recovery for a long time this way? It seems to me that
we'd need to actually trigger RequestXLogStreaming() in this case.

Greetings,

Andres Freund



Commits

  1. Allow walreceiver configuration to change on reload