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
