Hi, On 2017-04-22 17:53:19 +0200, Petr Jelinek wrote: > Here is patch. I changed both SIGINT and SIGUSR1 handlers, afaics it > does not break anything for existing walsender usage.
> diff --git a/src/backend/replication/logical/launcher.c > b/src/backend/replication/logical/launcher.c > index 761fbfa..e9dd886 100644 > --- a/src/backend/replication/logical/launcher.c > +++ b/src/backend/replication/logical/launcher.c > @@ -254,7 +254,7 @@ logicalrep_worker_launch(Oid dbid, Oid subid, const char > *subname, Oid userid, > BackgroundWorker bgw; > BackgroundWorkerHandle *bgw_handle; > int i; > - int slot; > + int slot = 0; > LogicalRepWorker *worker = NULL; > int nsyncworkers = 0; > TimestampTz now = GetCurrentTimestamp(); That seems independent and unnecessary? > -/* SIGUSR1: set flag to send WAL records */ > -static void > -WalSndXLogSendHandler(SIGNAL_ARGS) > -{ > - int save_errno = errno; > - > - latch_sigusr1_handler(); > - > - errno = save_errno; > -} > pqsignal(SIGPIPE, SIG_IGN); > - pqsignal(SIGUSR1, WalSndXLogSendHandler); /* request WAL sending > */ > + pqsignal(SIGUSR1, procsignal_sigusr1_handler); Those aren't actually entirely equivalent. I'm not sure it matters much, but WalSndXLogSendHandler didn't include a SetLatch(), but procsignal_sigusr1_handler() does. Normally redundant SetLatch() calls will just be elided, but what if walsender already woke up and did it's work? I think it'd be better to have PostgresMain() set up signals by default, and then have WalSndSignals() overwrites the ones it needs separate. 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? Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers