Re: allow online change primary_conninfo

Hi,

On 2019-02-16 14:50:35 +0300, Sergei Kornilov wrote:
> > I don't quite think this is the right design. IMO the startup process
> > should signal the walreceiver to shut down, and the wal receiver should
> > never look at the config.
> 
> Ok, i can rewrite such way. Please check attached version.
> 
> > Otherwise there's chances for knowledge of
> > pg.conf to differ between the processes.
> 
> I still not understood why this:
> - is not issue for startup process with all primary_conninfo logic
> - but issue for walreceiver with Michael Paquier patch; therefore without any primary_conninfo change logic in startup.
> In both cases primary_conninfo change logic is only in one process.
> 
> regards, Sergei

For one, it's not ok to just let the startup process think that the
walreceiver failed - that'll make it change source of WAL to the next
available method. Something we definitely do not want, as
restore_command is very commonly slower.

But just as importantly, I think, is that we ought to automate
cluster-wide tasks like failing over more inside postgres. And that's
much harder if different parts of PG, say the startup process and
walreceiver, have different assumptions about the current configuration.



> +void
> +ProcessStartupSigHup(void)
> +{
> +	char	   *conninfo = pstrdup(PrimaryConnInfo);
> +	char	   *slotname = pstrdup(PrimarySlotName);
> +
> +	ProcessConfigFile(PGC_SIGHUP);
> +
> +	/*
> +	 * we need shutdown running walreceiver if replication settings was
> +	 * changed. walreceiver will be started with new settings in next
> +	 * WaitForWALToBecomeAvailable iteration
> +	 */
> +	if ((strcmp(conninfo, PrimaryConnInfo) != 0 ||
> +		 strcmp(slotname, PrimarySlotName) != 0) &&
> +		WalRcvRunning())
> +	{
> +		ereport(LOG,
> +				(errmsg("terminating walreceiver process due to change of %s",
> +						strcmp(conninfo, PrimaryConnInfo) != 0 ? "primary_conninfo" : "primary_slot_name"),
> +				 errdetail("In a moment starts streaming WAL with new configuration.")));
> +
> +		ShutdownWalRcv();
> +		lastSourceFailed = true;
> +	}
> +
> +	pfree(conninfo);
> +	pfree(slotname);
> +}

That'd still switch to a different method, something we do not want...


Greetings,

Andres Freund