From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Unportable implementation of background worker start |
Date: | 2017-04-22 03:50:41 |
Message-ID: | 9205.1492833041@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I wrote:
> Attached is a lightly-tested draft patch that converts the postmaster to
> use a WaitEventSet for waiting in ServerLoop. I've got mixed emotions
> about whether this is the direction to proceed, though.
Attached are a couple of patches that represent a plausible Plan B.
The first one changes the postmaster to run its signal handlers without
specifying SA_RESTART. I've confirmed that that seems to fix the
select_parallel-test-takes-a-long-time problem on gaur/pademelon.
The second one uses pselect, if available, to replace the unblock-signals/
select()/block-signals dance in ServerLoop. On platforms where pselect
exists and works properly, that should fix the race condition I described
previously. On platforms where it doesn't, we're no worse off than
before.
As mentioned in the comments for the second patch, even if we don't
have working pselect(), the only problem is that ServerLoop's response
to an interrupt might be delayed by as much as the up-to-1-minute timeout.
The only existing case where that's really bad is launching multiple
bgworkers. I would therefore advocate also changing maybe_start_bgworker
to start up to N bgworkers per call, where N is large enough to pretty
much always satisfy simultaneously-arriving requests. I'd pick 100 or
so, but am willing to negotiate.
I think that these patches represent something we could back-patch
without a lot of trepidation, unlike the WaitEventSet-based approach.
Therefore, my proposal is to apply and backpatch these changes, and
call it good for v10. For v11, we could work on changing the postmaster
to not do work in signal handlers, as discussed upthread. That would
supersede these two patches completely, though I'd still advocate for
keeping the change in maybe_start_bgworker.
Note: for testing purposes, these patches are quite independent; just
ignore the hunk in the second patch that changes a comment added by
the first one.
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
no-sa-restart.patch | text/x-diff | 4.7 KB |
use-pselect-if-available.patch | text/x-diff | 7.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fabien COELHO | 2017-04-22 04:58:34 | Re: pgbench - allow to store select results into variables |
Previous Message | Craig Ringer | 2017-04-22 01:22:09 | Re: Old versions of Test::More |