Avoid calling kill() in a postmaster signal handler.
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Mon, 24 Aug 2009 17:23:02 +0000 (17:23 +0000)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Mon, 24 Aug 2009 17:23:02 +0000 (17:23 +0000)
This causes problems when the system load is high, per report from Zdenek
Kotala in <1250860954.1239.114.camel@localhost>; instead of calling kill
directly, have the signal handler set a flag which is checked in ServerLoop.
This way, the handler can return before being called again by a subsequent
signal sent from the autovacuum launcher.  Also, increase the sleep in the
launcher in this failure path to 1 second.

Backpatch to 8.3, which is when the signalling between autovacuum
launcher/postmaster was introduced.

Also, add a couple of ReleasePostmasterChildSlot calls in error paths; this
part backpatched to 8.4 which is when the child slot stuff was introduced.

src/backend/postmaster/autovacuum.c
src/backend/postmaster/postmaster.c

index 00d58f0bda3f09109bae99b940297c8047c98264..49b3ffa38cc384ebd29c4dae75fe39cbca253507 100644 (file)
@@ -55,7 +55,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.101 2009/08/12 20:53:30 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.102 2009/08/24 17:23:02 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -653,7 +653,7 @@ AutoVacLauncherMain(int argc, char *argv[])
                 * of a worker will continue to fail in the same way.
                 */
                AutoVacuumShmem->av_signal[AutoVacForkFailed] = false;
-               pg_usleep(100000L);     /* 100ms */
+               pg_usleep(1000000L);        /* 1s */
                SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_WORKER);
                continue;
            }
index e21d8e885b2227eff478732b62c26dc3d1164534..59a614aae2bd96a84887d6211fb91b5d783503b5 100644 (file)
@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.587 2009/08/07 05:58:55 heikki Exp $
+ *   $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.588 2009/08/24 17:23:02 alvherre Exp $
  *
  * NOTES
  *
@@ -290,6 +290,8 @@ bool        redirection_done = false;   /* stderr redirected for syslogger? */
 
 /* received START_AUTOVAC_LAUNCHER signal */
 static volatile sig_atomic_t start_autovac_launcher = false;
+/* the launcher needs to be signalled to communicate some condition */
+static volatile bool       avlauncher_needs_signal = false;
 
 /*
  * State for assigning random salts and cancel keys.
@@ -1391,6 +1393,14 @@ ServerLoop(void)
        if (PgStatPID == 0 && pmState == PM_RUN)
            PgStatPID = pgstat_start();
 
+       /* If we need to signal the autovacuum launcher, do so now */
+       if (avlauncher_needs_signal)
+       {
+           avlauncher_needs_signal = false;
+           if (AutoVacPID != 0)
+               kill(AutoVacPID, SIGUSR1);
+       }
+
        /*
         * Touch the socket and lock file every 58 minutes, to ensure that
         * they are not removed by overzealous /tmp-cleaning tasks.  We assume
@@ -3014,6 +3024,7 @@ BackendStartup(Port *port)
        /* in parent, fork failed */
        int         save_errno = errno;
 
+       (void) ReleasePostmasterChildSlot(bn->child_slot);
        free(bn);
        errno = save_errno;
        ereport(LOG,
@@ -4343,6 +4354,7 @@ StartAutovacuumWorker(void)
             * fork failed, fall through to report -- actual error message was
             * logged by StartAutoVacWorker
             */
+           (void) ReleasePostmasterChildSlot(bn->child_slot);
            free(bn);
        }
        else
@@ -4354,12 +4366,16 @@ StartAutovacuumWorker(void)
    /*
     * Report the failure to the launcher, if it's running.  (If it's not, we
     * might not even be connected to shared memory, so don't try to call
-    * AutoVacWorkerFailed.)
+    * AutoVacWorkerFailed.)  Note that we also need to signal it so that it
+    * responds to the condition, but we don't do that here, instead waiting
+    * for ServerLoop to do it.  This way we avoid a ping-pong signalling in
+    * quick succession between the autovac launcher and postmaster in case
+    * things get ugly.
     */
    if (AutoVacPID != 0)
    {
        AutoVacWorkerFailed();
-       kill(AutoVacPID, SIGUSR1);
+       avlauncher_needs_signal = true;
    }
 }