Re: Unportable implementation of background worker start

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

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org
Date: 2017-04-21T22:28:47Z
Lists: pgsql-hackers

Attachments

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> It also appears to me that do_start_bgworker's treatment of fork
>> failure is completely brain dead.  Did anyone really think about
>> that case?

> Hmm, I probably modelled it on autovacuum without giving that case much
> additional consideration.

Attached is a proposed patch that should make fork failure behave
sanely, ie it works much the same as a worker crash immediately after
launch.  I also refactored things a bit to make do_start_bgworker
fully responsible for updating the RegisteredBgWorker's state,
rather than doing just some of it as before.

I tested this by hot-wiring the fork_process call to fail some of
the time, which showed that the postmaster now seems to recover OK,
but parallel.c's logic is completely innocent of the idea that
worker-startup failure is possible.  The leader backend just freezes,
and nothing short of kill -9 on that backend will get you out of it.
Fixing that seems like material for a separate patch though.

			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.