Refactor how some aux processes advertise their ProcNumber

This moves the responsibility of setting the ProcGlobal->walwriterProc
and checkpointerProc fields into InitAuxiliaryProcess. Also switch to
the same pattern to advertise the autovacuum launcher's ProcNumber,
replacing the ad hoc av_launcherpid field in shared memory. This can
easily be extended to other aux processes in the future, if other
processes need to find them.

Switch to pg_atomic_uint32 for the fields. Seems easier to reason
about than volatile pointers. There was some precedence for that, as
were already using pg_atomic_uint32 for the procArrayGroupFirst and
clogGroupFirst fields, which also store ProcNumbers.

Todo: We could also replace WalRecv->procno with this, but that's a
little more code churn so I left that for the future.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/818bafaf-1e77-4c78-8037-d7120878d87c@iki.fi

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a81912b7441eef7e07a65366b2dd16d8dc6afe56..a8bbf6284a7fd01a42c7bebb3aad9aaa5ff07788 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2671,8 +2671,7 @@ XLogSetAsyncXactLSN(XLogRecPtr asyncXactLSN)
 
        if (wakeup)
        {
-               volatile PROC_HDR *procglobal = ProcGlobal;
-               ProcNumber      walwriterProc = procglobal->walwriterProc;
+               ProcNumber      walwriterProc = pg_atomic_read_u32(&ProcGlobal->walwriterProc);
 
                if (walwriterProc != INVALID_PROC_NUMBER)
                        SetLatch(&GetPGProcByNumber(walwriterProc)->procLatch);

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 127bd9e7da3ccd0be97ca9e06fe9d2ca59d13620..45abf48768afdaeffe0a6ec9805fdb2c0419d24e 100644 (file)
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -286,7 +286,6 @@ typedef struct AutoVacuumWorkItem
  * struct and the array of WorkerInfo structs.  This struct keeps:
  *
  * av_signal           set by other processes to indicate various conditions
- * av_launcherpid      the PID of the autovacuum launcher
  * av_freeWorkers      the WorkerInfo freelist
  * av_runningWorkers the WorkerInfo non-free queue
  * av_startingWorker pointer to WorkerInfo currently being started (cleared by
@@ -302,7 +301,6 @@ typedef struct AutoVacuumWorkItem
 typedef struct
 {
        sig_atomic_t av_signal[AutoVacNumSignals];
-       pid_t           av_launcherpid;
        dclist_head av_freeWorkers;
        dlist_head      av_runningWorkers;
        WorkerInfo      av_startingWorker;
@@ -602,8 +600,6 @@ AutoVacLauncherMain(const void *startup_data, size_t startup_data_len)
                proc_exit(0);                   /* done */
        }
 
-       AutoVacuumShmem->av_launcherpid = MyProcPid;
-
        /*
         * Create the initial database list.  The invariant we want this list to
         * keep is that it's ordered by decreasing next_worker.  As soon as an
@@ -837,8 +833,6 @@ AutoVacLauncherShutdown(void)
 {
        ereport(DEBUG1,
                        (errmsg_internal("autovacuum launcher shutting down")));
-       AutoVacuumShmem->av_launcherpid = 0;
-
        proc_exit(0);                           /* done */
 }
 
@@ -1569,6 +1563,8 @@ AutoVacWorkerMain(const void *startup_data, size_t startup_data_len)
         */
        if (AutoVacuumShmem->av_startingWorker != NULL)
        {
+               ProcNumber      launcherProc;
+
                MyWorkerInfo = AutoVacuumShmem->av_startingWorker;
                dbid = MyWorkerInfo->wi_dboid;
                MyWorkerInfo->wi_proc = MyProc;
@@ -1587,8 +1583,14 @@ AutoVacWorkerMain(const void *startup_data, size_t startup_data_len)
                on_shmem_exit(FreeWorkerInfo, 0);
 
                /* wake up the launcher */
-               if (AutoVacuumShmem->av_launcherpid != 0)
-                       kill(AutoVacuumShmem->av_launcherpid, SIGUSR2);
+               launcherProc = pg_atomic_read_u32(&ProcGlobal->avLauncherProc);
+               if (launcherProc != INVALID_PROC_NUMBER)
+               {
+                       int                     pid = GetPGProcByNumber(launcherProc)->pid;
+
+                       if (pid != 0)
+                               kill(pid, SIGUSR2);
+               }
        }
        else
        {
@@ -3559,7 +3561,6 @@ AutoVacuumShmemInit(void *arg)
 {
        WorkerInfo      worker;
 
-       AutoVacuumShmem->av_launcherpid = 0;
        dclist_init(&AutoVacuumShmem->av_freeWorkers);
        dlist_init(&AutoVacuumShmem->av_runningWorkers);
        AutoVacuumShmem->av_startingWorker = NULL;

diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 087120db0909d6ccaf7733e9a3ce104c794de7ad..a09d4097d51a8fccf34b5dd1a1b9db6b4af0e6d4 100644 (file)
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -47,6 +47,7 @@
 #include "libpq/pqsignal.h"
 #include "miscadmin.h"
 #include "pgstat.h"
+#include "port/atomics.h"
 #include "postmaster/auxprocess.h"
 #include "postmaster/bgwriter.h"
 #include "postmaster/interrupt.h"
@@ -358,12 +359,6 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len)
         */
        UpdateSharedMemoryConfig();
 
-       /*
-        * Advertise our proc number that backends can use to wake us up while
-        * we're sleeping.
-        */
-       ProcGlobal->checkpointerProc = MyProcNumber;
-
        /*
         * Loop until we've been asked to write the shutdown checkpoint or
         * terminate.
@@ -1120,7 +1115,7 @@ RequestCheckpoint(int flags)
        for (ntries = 0;; ntries++)
        {
                volatile PROC_HDR *procglobal = ProcGlobal;
-               ProcNumber      checkpointerProc = procglobal->checkpointerProc;
+               ProcNumber      checkpointerProc = pg_atomic_read_u32(&procglobal->checkpointerProc);
 
                if (checkpointerProc == INVALID_PROC_NUMBER)
                {
@@ -1261,8 +1256,7 @@ ForwardSyncRequest(const FileTag *ftag, SyncRequestType type)
        /* ... but not till after we release the lock */
        if (too_full)
        {
-               volatile PROC_HDR *procglobal = ProcGlobal;
-               ProcNumber      checkpointerProc = procglobal->checkpointerProc;
+               ProcNumber      checkpointerProc = pg_atomic_read_u32(&ProcGlobal->checkpointerProc);
 
                if (checkpointerProc != INVALID_PROC_NUMBER)
                        SetLatch(&GetPGProcByNumber(checkpointerProc)->procLatch);
@@ -1543,7 +1537,7 @@ void
 WakeupCheckpointer(void)
 {
        volatile PROC_HDR *procglobal = ProcGlobal;
-       ProcNumber      checkpointerProc = procglobal->checkpointerProc;
+       ProcNumber      checkpointerProc = pg_atomic_read_u32(&procglobal->checkpointerProc);
 
        if (checkpointerProc != INVALID_PROC_NUMBER)
                SetLatch(&GetPGProcByNumber(checkpointerProc)->procLatch);

diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index af24d05c542f8973f77f9fb7aac7e91c04156867..68dd5047c2070d2328b3df1a6705fb4f7aa0b40a 100644 (file)
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -206,12 +206,6 @@ WalWriterMain(const void *startup_data, size_t startup_data_len)
        hibernating = false;
        SetWalWriterSleeping(false);
 
-       /*
-        * Advertise our proc number that backends can use to wake us up while
-        * we're sleeping.
-        */
-       ProcGlobal->walwriterProc = MyProcNumber;
-
        /*
         * Loop forever
         */

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 87dd289f6268199a6bd348c49595b0e33f87bfb2..59640bb4f970cd07326026696782bf3127141ecd 100644 (file)
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -240,8 +240,9 @@ ProcGlobalShmemInit(void *arg)
        dlist_init(&ProcGlobal->bgworkerFreeProcs);
        dlist_init(&ProcGlobal->walsenderFreeProcs);
        ProcGlobal->startupBufferPinWaitBufId = -1;
-       ProcGlobal->walwriterProc = INVALID_PROC_NUMBER;
-       ProcGlobal->checkpointerProc = INVALID_PROC_NUMBER;
+       pg_atomic_init_u32(&ProcGlobal->avLauncherProc, INVALID_PROC_NUMBER);
+       pg_atomic_init_u32(&ProcGlobal->walwriterProc, INVALID_PROC_NUMBER);
+       pg_atomic_init_u32(&ProcGlobal->checkpointerProc, INVALID_PROC_NUMBER);
        pg_atomic_init_u32(&ProcGlobal->procArrayGroupFirst, INVALID_PROC_NUMBER);
        pg_atomic_init_u32(&ProcGlobal->clogGroupFirst, INVALID_PROC_NUMBER);
 
@@ -724,6 +725,14 @@ InitAuxiliaryProcess(void)
         */
        PGSemaphoreReset(MyProc->sem);
 
+       /* Some aux processes are also advertised in ProcGlobal */
+       if (MyBackendType == B_AUTOVAC_LAUNCHER)
+               pg_atomic_write_u32(&ProcGlobal->avLauncherProc, MyProcNumber);
+       if (MyBackendType == B_WAL_WRITER)
+               pg_atomic_write_u32(&ProcGlobal->walwriterProc, MyProcNumber);
+       if (MyBackendType == B_CHECKPOINTER)
+               pg_atomic_write_u32(&ProcGlobal->checkpointerProc, MyProcNumber);
+
        /*
         * Arrange to clean up at process exit.
         */
@@ -1101,6 +1110,25 @@ AuxiliaryProcKill(int code, Datum arg)
        SwitchBackToLocalLatch();
        pgstat_reset_wait_event_storage();
 
+       /*
+        * If this was one of the aux processes advertised in ProcGlobal, clear it
+        */
+       if (MyBackendType == B_AUTOVAC_LAUNCHER)
+       {
+               Assert(pg_atomic_read_u32(&ProcGlobal->avLauncherProc) == MyProcNumber);
+               pg_atomic_write_u32(&ProcGlobal->avLauncherProc, INVALID_PROC_NUMBER);
+       }
+       if (MyBackendType == B_WAL_WRITER)
+       {
+               Assert(pg_atomic_read_u32(&ProcGlobal->walwriterProc) == MyProcNumber);
+               pg_atomic_write_u32(&ProcGlobal->walwriterProc, INVALID_PROC_NUMBER);
+       }
+       if (MyBackendType == B_CHECKPOINTER)
+       {
+               Assert(pg_atomic_read_u32(&ProcGlobal->checkpointerProc) == MyProcNumber);
+               pg_atomic_write_u32(&ProcGlobal->checkpointerProc, INVALID_PROC_NUMBER);
+       }
+
        proc = MyProc;
        MyProc = NULL;
        MyProcNumber = INVALID_PROC_NUMBER;

diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 3e1d1fad5f9a497d47735f9e63b9fdfe3c97c706..0314f979e60575243e27af3160b5e025a36837e0 100644 (file)
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -488,11 +488,12 @@ typedef struct PROC_HDR
        pg_atomic_uint32 clogGroupFirst;
 
        /*
-        * Current slot numbers of some auxiliary processes. There can be only one
+        * Current proc numbers of some auxiliary processes. There can be only one
         * of each of these running at a time.
         */
-       ProcNumber      walwriterProc;
-       ProcNumber      checkpointerProc;
+       pg_atomic_uint32 avLauncherProc;
+       pg_atomic_uint32 walwriterProc;
+       pg_atomic_uint32 checkpointerProc;
 
        /* Current shared estimate of appropriate spins_per_delay value */
        int                     spins_per_delay;