Report failure to start a background worker.

When a worker is flagged as BGW_NEVER_RESTART and we fail to start it,
or if it is not marked BGW_NEVER_RESTART but is terminated before
startup succeeds, what BgwHandleStatus should be reported?  The
previous code really hadn't considered this possibility (as indicated
by the comments which ignore it completely) and would typically return
BGWH_NOT_YET_STARTED, but that's not a good answer, because then
there's no way for code using GetBackgroundWorkerPid() to tell the
difference between a worker that has not started but will start
later and a worker that has not started and will never be started.
So, when this case happens, return BGWH_STOPPED instead.  Update the
comments to reflect this.

The preceding fix by itself is insufficient to fix the problem,
because the old code also didn't send a notification to the process
identified in bgw_notify_pid when startup failed.  That might've
been technically correct under the theory that the status of the
worker was BGWH_NOT_YET_STARTED, because the status would indeed not
change when the worker failed to start, but now that we're more
usefully reporting BGWH_STOPPED, a notification is needed.

Without these fixes, code which starts background workers and then
uses the recommended APIs to wait for those background workers to
start would hang indefinitely if the postmaster failed to fork a
worker.

Amit Kapila and Robert Haas

Discussion: http://postgr.es/m/CAA4eK1KDfKkvrjxsKJi3WPyceVi3dH1VCkbTJji2fuwKuB=3uw@mail.gmail.com

diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index f752a12713c25da2375f700ca887f263054db0dc..88806ebe6b3410964085b9d883ad2cb5ffe16743 100644 (file)
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -1034,14 +1034,18 @@ RegisterDynamicBackgroundWorker(BackgroundWorker *worker,
  * Get the PID of a dynamically-registered background worker.
  *
  * If the worker is determined to be running, the return value will be
- * BGWH_STARTED and *pidp will get the PID of the worker process.
- * Otherwise, the return value will be BGWH_NOT_YET_STARTED if the worker
- * hasn't been started yet, and BGWH_STOPPED if the worker was previously
- * running but is no longer.
+ * BGWH_STARTED and *pidp will get the PID of the worker process.  If the
+ * postmaster has not yet attempted to start the worker, the return value will
+ * be BGWH_NOT_YET_STARTED.  Otherwise, the return value is BGWH_STOPPED.
  *
- * In the latter case, the worker may be stopped temporarily (if it is
- * configured for automatic restart and exited non-zero) or gone for
- * good (if it exited with code 0 or if it is configured not to restart).
+ * BGWH_STOPPED can indicate either that the worker is temporarily stopped
+ * (because it is configured for automatic restart and exited non-zero),
+ * or that the worker is permanently stopped (because it exited with exit
+ * code 0, or was not configured for automatic restart), or even that the
+ * worker was unregistered without ever starting (either because startup
+ * failed and the worker is not configured for automatic restart, or because
+ * TerminateBackgroundWorker was used before the worker was successfully
+ * started).
  */
 BgwHandleStatus
 GetBackgroundWorkerPid(BackgroundWorkerHandle *handle, pid_t *pidp)
@@ -1066,8 +1070,11 @@ GetBackgroundWorkerPid(BackgroundWorkerHandle *handle, pid_t *pidp)
         * time, but we assume such changes are atomic.  So the value we read
         * won't be garbage, but it might be out of date by the time the caller
         * examines it (but that's unavoidable anyway).
+        *
+        * The in_use flag could be in the process of changing from true to false,
+        * but if it is already false then it can't change further.
         */
-       if (handle->generation != slot->generation)
+       if (handle->generation != slot->generation || !slot->in_use)
                pid = 0;
        else
                pid = slot->pid;

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 94a46093713a6bd0dd23c6561fb9fd505f3ff15d..17c7f7e78fe7fd47cd411ad16db56cf4e9fa40e6 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5909,7 +5909,16 @@ maybe_start_bgworkers(void)
                {
                        if (rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART)
                        {
+                               int                     notify_pid;
+
+                               notify_pid = rw->rw_worker.bgw_notify_pid;
+
                                ForgetBackgroundWorker(&iter);
+
+                               /* Report worker is gone now. */
+                               if (notify_pid != 0)
+                                       kill(notify_pid, SIGUSR1);
+
                                continue;
                        }