Re: allow online change primary_conninfo

On Wed, Mar 25, 2020 at 09:08:12PM -0300, Alvaro Herrera wrote:
> ... as in the attached version.

Patch 0001 is one that has already been discussed previously (thanks
for keeping the extra comments about GUCs and WAL receivers).  That
looks fine to me.

> Sergei included LOG messages to indicate which setting has been changed.
> I put these in "#if 0" for now, but I'm thinking to remove these
> altogether; we already have LOG messages when a setting changes value,
> per ProcessConfigFileInternal(); the log sequence looks like this, taken
> from tmp_check/log/001_stream_rep_standby_2.log after running the tests:

Yes, it makes sense to remove any knowledge of GUC updates from
StartupRequestWalReceiverRestart().  I would add a description at the
top of this routine by the way.

+extern void ProcessStartupSigHup(void);
This is declared, but used nowhere in patch 0002.

Wouldn't it be better to be more careful about the NULL-ness of the
string parameters in StartupRereadConfig()?

+   if (!slotnameChanged && strcmp(PrimarySlotName, "") == 0)
+       tempSlotChanged = tempSlot != wal_receiver_create_temp_slot;
I would add parens here for clarity.

> which looks sufficient.  Maybe we can reword that new message, say "wal
> receiver process shutdown forced by parameter change".  Not sure if we
> can or should adjust the FATAL line; probably not worth the trouble.

There is merit to keep the LOG in StartupRequestWalReceiverRestart()
unchanged, as the routine may get called in the future in some other
context.
--
Michael