Horton

Re: Unportable implementation of background worker start

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Rémi Zara <remi_zara@mac.com>, CM Team <cm@enterprisedb.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-04-27T01:38:38Z
Lists: pgsql-hackers 
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Apr 26, 2017 at 8:37 PM, Andres Freund <andres@anarazel.de> wrote:
>>> 3. Go ahead with converting the postmaster to use WaitEventSet, a la
>>> the draft patch I posted earlier.  I'd be happy to do this if we were
>>> at the start of a devel cycle, but right now seems a bit late --- not
>>> to mention that we really need to fix 9.6 as well.

>> Yea, backpatching this to 9.6 seems like a bigger hammer than
>> appropriate.  I'm on the fence WRT master, I think there's an argument
>> to be made that this is going to become a bigger and bigger problem, and
>> that we'll wish in a year or two that we had fewer releases with
>> parallelism etc that don't use WaitEventSets.

> I think changing this might be wise.  This problem isn't going away
> for real until we do this, right?

Sure, but we have a lot of other problems that aren't going away until
we fix them, either.  This patch smells like new development to me ---
and it's not even very complete, because really what Andres wants to do
(and I concur) is to get rid of the postmaster's habit of doing
interesting things in signal handlers.  I'm definitely not on board with
doing that for v10 at this point.  But if we apply this patch, and then
do that in v11, then v10 will look like neither earlier nor later branches
with respect to the postmaster's event wait mechanisms.  I think that's
a recipe for undue maintenance pain.

I believe that the already-committed patches represent a sufficient-for-now
response to the known performance problems here, and so I'm thinking we
should stop here for v10.  I'm okay with pushing the latch.c changes
I just proposed, because those provide a useful safeguard against
extension modules doing something exciting in the postmaster process.
But I don't think we should go much further than that for v10.

			regards, tom lane

Commits

  1. Cope with glibc too old to have epoll_create1().

  2. Make latch.c more paranoid about child-process cases.

  3. Allow multiple bgworkers to be launched per postmaster iteration.

  4. Revert "Use pselect(2) not select(2), if available, to wait in postmaster's loop."

  5. Use pselect(2) not select(2), if available, to wait in postmaster's loop.

  6. Run the postmaster's signal handlers without SA_RESTART.

  7. Fix postmaster's handling of fork failure for a bgworker process.

  8. Partially revert commit 536d47bd9d5fce8d91929bee3128fa1d08dbcc57.

  9. Avoid depending on non-POSIX behavior of fcntl(2).

  10. Remove long-obsolete catering for platforms without F_SETFD/FD_CLOEXEC.