On 2017-04-24 04:26:16 +0200, Petr Jelinek wrote:
> > WalSndLastCycleHandler is genuinely different. WalSndSigHupHandler
> > currently sets a different variable from postgres.c, but that seems like
> > a bad idea, because afaics we'll plainly ignore SIGHUPS unless in
> > WalSndLoop, WalSndWriteData,WalSndWaitForWal. That actually seems like
> > an active bug to me?
> >
>
> Hmm I don't think good solution is to use same variable though,
> walsender needs to do slightly different thing on SIGHUP,
Hm? You mean SyncRepInitConfig()? I don't think that matters, because
it's only needed if config is changed while streaming.
> plus the
> got_SIGHUP is not the type of variable we want to export. I wonder if it
> would be enough to just add check for got_SIGHUP somewhere at the
> beginning of exec_replication_command().
I don't think that'd be sufficient, because we really want config
changes to get picked up while idle, and that won't work from within
exec_replication_command(). And that pretty much means it'll have to
happen outside of walsender.c.
FWIW, while it's not pretty, I actually see very little reason not to
share got_SIGHUP (with a bit less generic name name) between different
types of processes (i.e. putting it in miscadmin.h or such). It's not
exactly pretty, but there's also no benefit in duplicating it
everywhere, and without it you run into the issue presented here.
Greetings,
Andres Freund
