Re: Connection limits/permissions, slotsync workers, etc - Mailing list pgsql-hackers

"Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com> writes:
> On Thursday, December 26, 2024 3:50 AM Tom Lane <tgl@sss.pgh.pa.us>
>> I wonder if the AV launcher and slotsync worker could be reclassified as "auxiliary
>> processes" instead of being their own weird animal.

> It appears that the current aux processes do not run transactions as stated in the
> comments[1], so we may need to somehow release this restriction to achieve the
> goal.

Ah, right, I'd forgotten about that restriction.  I agree that
removing it wouldn't be very reasonable.  However, I still would
rather avoid making the slotsync worker be its very own special
snowflake, because that offers no support for the next person
who wants to invent a new sort of specialized transaction-capable
process.

Attached is an alternative proposal that groups the autovac launcher
and slotsync worker into a new category of "special workers" (better
name welcome).  I chose to put them into the existing autovacFreeProcs
freelist, partly because the autovac launcher lives there already
but mostly because I don't want to add another freelist in a patch
we need to put into v17.  (As written, your patch is an ABI break.
It'd probably be safe to add a new freelist at the end of the struct
in v17, but I'm a little shy about that in view of recent bugs.  In
any case, a freelist having at most two members seems rather silly.)

I was amused but not terribly surprised to notice that the comments
in InitProcGlobal were *already* out of date, in that they didn't
account for the walsender PGPROC pool.  We have a remarkably bad
track record for updating comments that are more than about two
lines away from the code they describe :-(

Thoughts?

            regards, tom lane

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 720ef99ee8..10d4fb4ea1 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -197,11 +197,12 @@ InitProcGlobal(void)

     /*
      * Create and initialize all the PGPROC structures we'll need.  There are
-     * five separate consumers: (1) normal backends, (2) autovacuum workers
-     * and the autovacuum launcher, (3) background workers, (4) auxiliary
-     * processes, and (5) prepared transactions.  Each PGPROC structure is
-     * dedicated to exactly one of these purposes, and they do not move
-     * between groups.
+     * six separate consumers: (1) normal backends, (2) autovacuum workers and
+     * special workers, (3) background workers, (4) walsenders, (5) auxiliary
+     * processes, and (6) prepared transactions.  (For largely-historical
+     * reasons, we combine autovacuum and special workers into one category
+     * with a single freelist.)  Each PGPROC structure is dedicated to exactly
+     * one of these purposes, and they do not move between groups.
      */
     procs = (PGPROC *) ShmemAlloc(TotalProcs * sizeof(PGPROC));
     MemSet(procs, 0, TotalProcs * sizeof(PGPROC));
@@ -269,12 +270,13 @@ InitProcGlobal(void)
         }

         /*
-         * Newly created PGPROCs for normal backends, autovacuum and bgworkers
-         * must be queued up on the appropriate free list.  Because there can
-         * only ever be a small, fixed number of auxiliary processes, no free
-         * list is used in that case; InitAuxiliaryProcess() instead uses a
-         * linear search.   PGPROCs for prepared transactions are added to a
-         * free list by TwoPhaseShmemInit().
+         * Newly created PGPROCs for normal backends, autovacuum workers,
+         * special workers, bgworkers, and walsenders must be queued up on the
+         * appropriate free list.  Because there can only ever be a small,
+         * fixed number of auxiliary processes, no free list is used in that
+         * case; InitAuxiliaryProcess() instead uses a linear search.  PGPROCs
+         * for prepared transactions are added to a free list by
+         * TwoPhaseShmemInit().
          */
         if (i < MaxConnections)
         {
@@ -282,13 +284,13 @@ InitProcGlobal(void)
             dlist_push_tail(&ProcGlobal->freeProcs, &proc->links);
             proc->procgloballist = &ProcGlobal->freeProcs;
         }
-        else if (i < MaxConnections + autovacuum_max_workers + 1)
+        else if (i < MaxConnections + autovacuum_max_workers + NUM_SPECIAL_WORKER_PROCS)
         {
-            /* PGPROC for AV launcher/worker, add to autovacFreeProcs list */
+            /* PGPROC for AV or special worker, add to autovacFreeProcs list */
             dlist_push_tail(&ProcGlobal->autovacFreeProcs, &proc->links);
             proc->procgloballist = &ProcGlobal->autovacFreeProcs;
         }
-        else if (i < MaxConnections + autovacuum_max_workers + 1 + max_worker_processes)
+        else if (i < MaxConnections + autovacuum_max_workers + NUM_SPECIAL_WORKER_PROCS + max_worker_processes)
         {
             /* PGPROC for bgworker, add to bgworkerFreeProcs list */
             dlist_push_tail(&ProcGlobal->bgworkerFreeProcs, &proc->links);
@@ -358,8 +360,11 @@ InitProcess(void)
     if (IsUnderPostmaster)
         RegisterPostmasterChildActive();

-    /* Decide which list should supply our PGPROC. */
-    if (AmAutoVacuumLauncherProcess() || AmAutoVacuumWorkerProcess())
+    /*
+     * Decide which list should supply our PGPROC.  This logic must match the
+     * way the freelists were constructed in InitProcGlobal().
+     */
+    if (AmAutoVacuumWorkerProcess() || AmSpecialWorkerProcess())
         procgloballist = &ProcGlobal->autovacFreeProcs;
     else if (AmBackgroundWorkerProcess())
         procgloballist = &ProcGlobal->bgworkerFreeProcs;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 770ab6906e..9731de5781 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -544,9 +544,9 @@ InitializeMaxBackends(void)
 {
     Assert(MaxBackends == 0);

-    /* the extra unit accounts for the autovacuum launcher */
-    MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
-        max_worker_processes + max_wal_senders;
+    /* Note that this does not include "auxiliary" processes */
+    MaxBackends = MaxConnections + autovacuum_max_workers +
+        max_worker_processes + max_wal_senders + NUM_SPECIAL_WORKER_PROCS;

     if (MaxBackends > MAX_BACKENDS)
         ereport(ERROR,
@@ -555,7 +555,7 @@ InitializeMaxBackends(void)
                  errdetail("\"max_connections\" (%d) plus \"autovacuum_max_workers\" (%d) plus
\"max_worker_processes\"(%d) plus \"max_wal_senders\" (%d) must be less than %d.", 
                            MaxConnections, autovacuum_max_workers,
                            max_worker_processes, max_wal_senders,
-                           MAX_BACKENDS)));
+                           MAX_BACKENDS - (NUM_SPECIAL_WORKER_PROCS - 1))));
 }

 /*
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 3f97fcef80..d07181f932 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -350,8 +350,9 @@ typedef enum BackendType

     /*
      * Auxiliary processes. These have PGPROC entries, but they are not
-     * attached to any particular database. There can be only one of each of
-     * these running at a time.
+     * attached to any particular database, and cannot run transactions or
+     * even take heavyweight locks. There can be only one of each of these
+     * running at a time.
      *
      * If you modify these, make sure to update NUM_AUXILIARY_PROCS and the
      * glossary in the docs.
@@ -388,6 +389,10 @@ extern PGDLLIMPORT BackendType MyBackendType;
 #define AmWalSummarizerProcess()    (MyBackendType == B_WAL_SUMMARIZER)
 #define AmWalWriterProcess()        (MyBackendType == B_WAL_WRITER)

+#define AmSpecialWorkerProcess() \
+    (AmAutoVacuumLauncherProcess() || \
+     AmLogicalSlotSyncWorkerProcess())
+
 extern const char *GetBackendTypeDesc(BackendType backendType);

 extern void SetDatabasePath(const char *path);
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 5a3dd5d2d4..14e34d4e93 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -408,7 +408,7 @@ typedef struct PROC_HDR
     uint32        allProcCount;
     /* Head of list of free PGPROC structures */
     dlist_head    freeProcs;
-    /* Head of list of autovacuum's free PGPROC structures */
+    /* Head of list of autovacuum & special worker free PGPROC structures */
     dlist_head    autovacFreeProcs;
     /* Head of list of bgworker free PGPROC structures */
     dlist_head    bgworkerFreeProcs;
@@ -442,9 +442,19 @@ extern PGDLLIMPORT PGPROC *PreparedXactProcs;
 #define GetPGProcByNumber(n) (&ProcGlobal->allProcs[(n)])
 #define GetNumberFromPGProc(proc) ((proc) - &ProcGlobal->allProcs[0])

+/*
+ * We set aside some extra PGPROC structures for "special worker" processes,
+ * which are full-fledged backends (they can run transactions)
+ * but are unique animals that there's never more than one of.
+ * Currently there are two such processes: the autovacuum launcher
+ * and the slotsync worker.
+ */
+#define NUM_SPECIAL_WORKER_PROCS    2
+
 /*
  * We set aside some extra PGPROC structures for auxiliary processes,
- * ie things that aren't full-fledged backends but need shmem access.
+ * ie things that aren't full-fledged backends (they cannot run transactions
+ * or take heavyweight locks) but need shmem access.
  *
  * Background writer, checkpointer, WAL writer, WAL summarizer, and archiver
  * run during normal operation.  Startup process and WAL receiver also consume