From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> |
Cc: | Craig Ringer <craig(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net> |
Subject: | Re: walsender & parallelism |
Date: | 2017-04-23 23:59:41 |
Message-ID: | 20170423235941.qosiuoyqprq4nu7v@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Noah Misch | 2017-04-24 00:02:03 | Re: Quorum commit for multiple synchronous replication. |
Previous Message | Michael Paquier | 2017-04-23 23:58:44 | Re: A note about debugging TAP failures |