Fix management of pendingOpsTable in auxiliary processes.

mdinit() was misusing IsBootstrapProcessingMode() to decide whether to
create an fsync pending-operations table in the current process.  This led
to creating a table not only in the startup and checkpointer processes as
intended, but also in the bgwriter process, not to mention other auxiliary
processes such as walwriter and walreceiver.  Creation of the table in the
bgwriter is fatal, because it absorbs fsync requests that should have gone
to the checkpointer; instead they just sit in bgwriter local memory and are
never acted on.  So writes performed by the bgwriter were not being fsync'd
which could result in data loss after an OS crash.  I think there is no
live bug with respect to walwriter and walreceiver because those never
perform any writes of shared buffers; but the potential is there for
future breakage in those processes too.

To fix, make AuxiliaryProcessMain() export the current process's
AuxProcType as a global variable, and then make mdinit() test directly for
the types of aux process that should have a pendingOpsTable.  Having done
that, we might as well also get rid of the random bool flags such as
am_walreceiver that some of the aux processes had grown.  (Note that we
could not have fixed the bug by examining those variables in mdinit(),
because it's called from BaseInit() which is run by AuxiliaryProcessMain()
before entering any of the process-type-specific code.)

Back-patch to 9.2, where the problem was introduced by the split-up of
bgwriter and checkpointer processes.  The bogus pendingOpsTable exists
in walwriter and walreceiver processes in earlier branches, but absent
any evidence that it causes actual problems there, I'll leave the older
branches alone.

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3f3f9a3727b1c83e75d40b35fe447a18bf12d13b..a73cd6b9dec1a44f37842627e6c46cf3c05d3112 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8946,7 +8946,7 @@ get_sync_bit(int method)
         * after its written. Also, walreceiver performs unaligned writes, which
         * don't work with O_DIRECT, so it is required for correctness too.
         */
-       if (!XLogIsNeeded() && !am_walreceiver)
+       if (!XLogIsNeeded() && !AmWalReceiverProcess())
                o_direct_flag = PG_O_DIRECT;
 
        switch (method)

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index e3ae92d540d10d5f38b03304de1cb25e2b823beb..b7428829f372330227eda53c858f47c467a45f35 100644 (file)
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -63,6 +63,8 @@ static void cleanup(void);
  * ----------------
  */
 
+AuxProcType    MyAuxProcType = NotAnAuxProcess;        /* declared in miscadmin.h */
+
 Relation       boot_reldesc;           /* current relation descriptor */
 
 Form_pg_attribute attrtypes[MAXATTR];  /* points to attribute info */
@@ -187,7 +189,6 @@ AuxiliaryProcessMain(int argc, char *argv[])
 {
        char       *progname = argv[0];
        int                     flag;
-       AuxProcType auxType = CheckerProcess;
        char       *userDoption = NULL;
 
        /*
@@ -228,6 +229,9 @@ AuxiliaryProcessMain(int argc, char *argv[])
                argc--;
        }
 
+       /* If no -x argument, we are a CheckerProcess */
+       MyAuxProcType = CheckerProcess;
+
        while ((flag = getopt(argc, argv, "B:c:d:D:Fr:x:-:")) != -1)
        {
                switch (flag)
@@ -258,7 +262,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
                                strlcpy(OutputFileName, optarg, MAXPGPATH);
                                break;
                        case 'x':
-                               auxType = atoi(optarg);
+                               MyAuxProcType = atoi(optarg);
                                break;
                        case 'c':
                        case '-':
@@ -308,7 +312,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
        {
                const char *statmsg;
 
-               switch (auxType)
+               switch (MyAuxProcType)
                {
                        case StartupProcess:
                                statmsg = "startup process";
@@ -374,15 +378,15 @@ AuxiliaryProcessMain(int argc, char *argv[])
                /*
                 * Assign the ProcSignalSlot for an auxiliary process.  Since it
                 * doesn't have a BackendId, the slot is statically allocated based on
-                * the auxiliary process type (auxType).  Backends use slots indexed
-                * in the range from 1 to MaxBackends (inclusive), so we use
+                * the auxiliary process type (MyAuxProcType).  Backends use slots
+                * indexed in the range from 1 to MaxBackends (inclusive), so we use
                 * MaxBackends + AuxProcType + 1 as the index of the slot for an
                 * auxiliary process.
                 *
                 * This will need rethinking if we ever want more than one of a
                 * particular auxiliary process type.
                 */
-               ProcSignalInit(MaxBackends + auxType + 1);
+               ProcSignalInit(MaxBackends + MyAuxProcType + 1);
 
                /* finish setting up bufmgr.c */
                InitBufferPoolBackend();
@@ -396,7 +400,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
         */
        SetProcessingMode(NormalProcessing);
 
-       switch (auxType)
+       switch (MyAuxProcType)
        {
                case CheckerProcess:
                        /* don't set signals, they're useless here */
@@ -436,7 +440,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
                        proc_exit(1);           /* should never return */
 
                default:
-                       elog(PANIC, "unrecognized process type: %d", auxType);
+                       elog(PANIC, "unrecognized process type: %d", (int) MyAuxProcType);
                        proc_exit(1);
        }
 }

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 5f93fccbfab1bbb8306f5de4ad228f3cb48b0862..f98f138e9a6b2ec0273c700b4944da2c57563af0 100644 (file)
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -74,11 +74,6 @@ int                  BgWriterDelay = 200;
 static volatile sig_atomic_t got_SIGHUP = false;
 static volatile sig_atomic_t shutdown_requested = false;
 
-/*
- * Private state
- */
-static bool am_bg_writer = false;
-
 /* Signal handlers */
 
 static void bg_quickdie(SIGNAL_ARGS);
@@ -90,8 +85,8 @@ static void bgwriter_sigusr1_handler(SIGNAL_ARGS);
 /*
  * Main entry point for bgwriter process
  *
- * This is invoked from BootstrapMain, which has already created the basic
- * execution environment, but not enabled signals yet.
+ * This is invoked from AuxiliaryProcessMain, which has already created the
+ * basic execution environment, but not enabled signals yet.
  */
 void
 BackgroundWriterMain(void)
@@ -100,8 +95,6 @@ BackgroundWriterMain(void)
        MemoryContext bgwriter_context;
        bool            prev_hibernate;
 
-       am_bg_writer = true;
-
        /*
         * If possible, make this process a group leader, so that the postmaster
         * can signal any child processes too.  (bgwriter probably never has any

diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 92fd4276cd1b3be81d1ac741f9f6ea09d241ea52..b8715ea6762ab85983f5a6dc88c09c89a9aa9979 100644 (file)
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -153,8 +153,6 @@ static volatile sig_atomic_t shutdown_requested = false;
 /*
  * Private state
  */
-static bool am_checkpointer = false;
-
 static bool ckpt_active = false;
 
 /* these values are valid when ckpt_active is true: */
@@ -185,8 +183,8 @@ static void ReqShutdownHandler(SIGNAL_ARGS);
 /*
  * Main entry point for checkpointer process
  *
- * This is invoked from BootstrapMain, which has already created the basic
- * execution environment, but not enabled signals yet.
+ * This is invoked from AuxiliaryProcessMain, which has already created the
+ * basic execution environment, but not enabled signals yet.
  */
 void
 CheckpointerMain(void)
@@ -195,7 +193,6 @@ CheckpointerMain(void)
        MemoryContext checkpointer_context;
 
        CheckpointerShmem->checkpointer_pid = MyProcPid;
-       am_checkpointer = true;
 
        /*
         * If possible, make this process a group leader, so that the postmaster
@@ -685,7 +682,7 @@ CheckpointWriteDelay(int flags, double progress)
        static int      absorb_counter = WRITES_PER_ABSORB;
 
        /* Do nothing if checkpoint is being executed by non-checkpointer process */
-       if (!am_checkpointer)
+       if (!AmCheckpointerProcess())
                return;
 
        /*
@@ -1129,7 +1126,7 @@ ForwardFsyncRequest(RelFileNode rnode, ForkNumber forknum, BlockNumber segno)
        if (!IsUnderPostmaster)
                return false;                   /* probably shouldn't even get here */
 
-       if (am_checkpointer)
+       if (AmCheckpointerProcess())
                elog(ERROR, "ForwardFsyncRequest must not be called in checkpointer");
 
        LWLockAcquire(CheckpointerCommLock, LW_EXCLUSIVE);
@@ -1306,7 +1303,7 @@ AbsorbFsyncRequests(void)
        CheckpointerRequest *request;
        int                     n;
 
-       if (!am_checkpointer)
+       if (!AmCheckpointerProcess())
                return;
 
        /*

diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index b7b85125553cf39bef2bc9e775a2e3c69fad8f98..8a7949558102f7c695595d6f48ea561228c7d4ab 100644 (file)
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -89,8 +89,8 @@ static void walwriter_sigusr1_handler(SIGNAL_ARGS);
 /*
  * Main entry point for walwriter process
  *
- * This is invoked from BootstrapMain, which has already created the basic
- * execution environment, but not enabled signals yet.
+ * This is invoked from AuxiliaryProcessMain, which has already created the
+ * basic execution environment, but not enabled signals yet.
  */
 void
 WalWriterMain(void)

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index fd119d38cbbc7745cb245e3158facc4f65ef2181..b0a8b197603906d8c876e084fd3674a8f5d1b280 100644 (file)
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -52,10 +52,8 @@
 #include "utils/resowner.h"
 #include "utils/timestamp.h"
 
-/* Global variable to indicate if this process is a walreceiver process */
-bool           am_walreceiver;
 
-/* GUC variable */
+/* GUC variables */
 int                    wal_receiver_status_interval;
 bool           hot_standby_feedback;
 
@@ -176,8 +174,6 @@ WalReceiverMain(void)
        /* use volatile pointer to prevent code rearrangement */
        volatile WalRcvData *walrcv = WalRcv;
 
-       am_walreceiver = true;
-
        /*
         * WalRcv should be set up already (if we are a backend, we inherit this
         * by fork() or EXEC_BACKEND mechanism from the postmaster).

diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 3d7e85fba470ff30aca615c0a8c64b503422f756..0f0d4dd48dffc394b368e675c5ac33f4db31bb84 100644 (file)
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -17,7 +17,6 @@
 #include <signal.h>
 #include <unistd.h>
 
-#include "bootstrap/bootstrap.h"
 #include "commands/async.h"
 #include "miscadmin.h"
 #include "storage/latch.h"

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index c05315d884978d8690a292ed1e0d3960bc52b5ae..f074cedca598d3afe8436c66fe097ca001441014 100644 (file)
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -196,11 +196,10 @@ mdinit(void)
 
        /*
         * Create pending-operations hashtable if we need it.  Currently, we need
-        * it if we are standalone (not under a postmaster) OR if we are a
-        * bootstrap-mode subprocess of a postmaster (that is, a startup or
-        * checkpointer process).
+        * it if we are standalone (not under a postmaster) or if we are a startup
+        * or checkpointer auxiliary process.
         */
-       if (!IsUnderPostmaster || IsBootstrapProcessingMode())
+       if (!IsUnderPostmaster || AmStartupProcess() || AmCheckpointerProcess())
        {
                HASHCTL         hash_ctl;
 

diff --git a/src/include/bootstrap/bootstrap.h b/src/include/bootstrap/bootstrap.h
index b31bca941101247da9e6c8756f08ba28631be85a..58fc2825b869a735aa4ded3da967cc13855c298b 100644 (file)
--- a/src/include/bootstrap/bootstrap.h
+++ b/src/include/bootstrap/bootstrap.h
@@ -16,18 +16,6 @@
 
 #include "nodes/execnodes.h"
 
-typedef enum
-{
-       CheckerProcess,
-       BootstrapProcess,
-       StartupProcess,
-       BgWriterProcess,
-       CheckpointerProcess,
-       WalWriterProcess,
-       WalReceiverProcess,
-
-       NUM_AUXPROCTYPES                        /* Must be last! */
-} AuxProcType;
 
 /*
  * MAXATTR is the maximum number of attributes in a relation supported

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index b186eed8f478ae66ca1fde3ee8f08e613d97c54d..8df2a28126a1f24e7a72513fb253da5c3f87773f 100644 (file)
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -328,8 +328,8 @@ extern bool superuser_arg(Oid roleid);      /* given user is superuser */
  * initialization is complete. Some code behaves differently when executed
  * in this mode to enable system bootstrapping.
  *
- * If a POSTGRES binary is in normal mode, then all code may be executed
- * normally.
+ * If a POSTGRES backend process is in normal mode, then all code may be
+ * executed normally.
  */
 
 typedef enum ProcessingMode
@@ -341,9 +341,11 @@ typedef enum ProcessingMode
 
 extern ProcessingMode Mode;
 
-#define IsBootstrapProcessingMode() ((bool)(Mode == BootstrapProcessing))
-#define IsInitProcessingMode() ((bool)(Mode == InitProcessing))
-#define IsNormalProcessingMode() ((bool)(Mode == NormalProcessing))
+#define IsBootstrapProcessingMode()    (Mode == BootstrapProcessing)
+#define IsInitProcessingMode()         (Mode == InitProcessing)
+#define IsNormalProcessingMode()       (Mode == NormalProcessing)
+
+#define GetProcessingMode() Mode
 
 #define SetProcessingMode(mode) \
        do { \
@@ -353,7 +355,35 @@ extern ProcessingMode Mode;
                Mode = (mode); \
        } while(0)
 
-#define GetProcessingMode() Mode
+
+/*
+ * Auxiliary-process type identifiers.  These used to be in bootstrap.h
+ * but it seems saner to have them here, with the ProcessingMode stuff.
+ * The MyAuxProcType global is defined and set in bootstrap.c.
+ */
+
+typedef enum
+{
+       NotAnAuxProcess = -1,
+       CheckerProcess = 0,
+       BootstrapProcess,
+       StartupProcess,
+       BgWriterProcess,
+       CheckpointerProcess,
+       WalWriterProcess,
+       WalReceiverProcess,
+
+       NUM_AUXPROCTYPES                        /* Must be last! */
+} AuxProcType;
+
+extern AuxProcType MyAuxProcType;
+
+#define AmBootstrapProcess()           (MyAuxProcType == BootstrapProcess)
+#define AmStartupProcess()                     (MyAuxProcType == StartupProcess)
+#define AmBackgroundWriterProcess()    (MyAuxProcType == BgWriterProcess)
+#define AmCheckpointerProcess()                (MyAuxProcType == CheckpointerProcess)
+#define AmWalWriterProcess()           (MyAuxProcType == WalWriterProcess)
+#define AmWalReceiverProcess()         (MyAuxProcType == WalReceiverProcess)
 
 
 /*****************************************************************************

diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h
index 31449d214ec1c0416ab5a4dbf6142b41903d8782..8ef67f8a867a9c0cf64dfefda294134453aff4e5 100644 (file)
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -17,7 +17,6 @@
 #include "storage/spin.h"
 #include "pgtime.h"
 
-extern bool am_walreceiver;
 extern int     wal_receiver_status_interval;
 extern bool hot_standby_feedback;