Re: allow online change primary_conninfo

On Mon, Nov 26, 2018 at 09:46:36AM -0800, Andres Freund wrote:
> I'm not sure I understand what you mean? The way I'd solve this is that
> that only walreceiver, at startup, writes out its conninfo/slot_name,
> sourcing the values from the GUCs. That way there's no issue with values
> being overwritten early.

WalRcv->conninfo, as stored in the WAL receiver, clobbers some of its
fields and includes a set of default parameters with its values after
establishing the connection so as no sensible data shows up in
pg_stat_wal_receiver so you cannot simply compare what is stored in the
WAL receiver with the GUCs to do the decision-making. For the password,
you'd want to do again a connection anyway even if the cloberred string
is the same.

What's proposed in the patch to issue an ERROR if the parameter has
changed does not look that bad to me as it depends on the process
context, but I think that this patch should document clearly that if
primary_conninfo or primary_slot_name are changed then the WAL receiver
is stopped immediately.

/*
- * replication slot name; is also used for walreceiver to connect with the
- * primary
+ * replication slot name used by runned walreceiver
*/
Not sure that there is much point in updating those comments, because
it still has the same meaning in the new context.

-RequestXLogStreaming(TimeLineID tli, XLogRecPtr recptr, const char *conninfo,
- const char *slotname)
+RequestXLogStreaming(TimeLineID tli, XLogRecPtr recptr)
That's perhaps a matter of taste, but keeping the current API as-is
looks cleaner to me, particularly because those fields get copied into
WalRcv, so I'd rather not change what is done in
WaitForWALToBecomeAvailable and keep the patch at its simplest shape.

+$node_standby_2->reload; # should have effect without restart
This does not wait for the change to be effective, so I think that you
introduce a race condition for slow machines with this test, making it
unstable.
--
Michael