On Wed, Jan 18, 2023 at 02:51:38PM -0500, Robert Haas wrote:
> Should (nfree < SuperuserReservedBackends) be using <=, or am I confused?

I believe < is correct.  At this point, the new backend will have already
claimed a proc struct, so if the number of remaining free slots equals the
number of reserved slots, it is okay.

> What's the deal with removing "and no new replication connections will
> be accepted" from the documentation? Is the existing documentation
> just wrong? If so, should we fix that first? And maybe delete
> "non-replication" from the error message that says "remaining
> connection slots are reserved for non-replication superuser
> connections"? It seems like right now the comments say that
> replication connections are a completely separate pool of connections,
> but the documentation and the error message make it sound otherwise.
> If that's true, then one of them is wrong, and I think it's the
> docs/error message. Or am I just misreading it?

I think you are right.  This seems to have been missed in ea92368.  I moved
this part to a new patch that should probably be back-patched to v12.

On that note, I wonder if it's worth changing the "sorry, too many clients
already" message to make it clear that max_connections has been reached.
IME some users are confused by this error, and I think it would be less
confusing if it pointed to the parameter that governs the number of
connection slots.  I'll create a new thread for this.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 8f0aa2fa54ae01149cffe9a69265f98e76a08a23 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandboss...@gmail.com>
Date: Wed, 18 Jan 2023 12:43:41 -0800
Subject: [PATCH v3 1/3] Code review for ea92368.

This commit missed an error message and a line in the docs.

Back-patch to v12.
---
 doc/src/sgml/config.sgml          | 3 +--
 src/backend/utils/init/postinit.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 89d53f2a64..e019a1aac9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -725,8 +725,7 @@ include_dir 'conf.d'
         number of active concurrent connections is at least
         <varname>max_connections</varname> minus
         <varname>superuser_reserved_connections</varname>, new
-        connections will be accepted only for superusers, and no
-        new replication connections will be accepted.
+        connections will be accepted only for superusers.
        </para>
 
        <para>
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index ae5a85ed65..9145d96b38 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -931,7 +931,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
 		!HaveNFreeProcs(ReservedBackends))
 		ereport(FATAL,
 				(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
-				 errmsg("remaining connection slots are reserved for non-replication superuser connections")));
+				 errmsg("remaining connection slots are reserved for superusers")));
 
 	/* Check replication permissions needed for walsender processes. */
 	if (am_walsender)
-- 
2.25.1

>From bc110461e4f0a73c2b76ba0a6e821349c2cbe3df Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandboss...@gmail.com>
Date: Tue, 17 Jan 2023 13:58:56 -0800
Subject: [PATCH v3 2/3] Rename ReservedBackends to SuperuserReservedBackends.

This is in preparation for adding a new reserved_connections GUC
that will use the ReservedBackends variable name.
---
 src/backend/postmaster/postmaster.c | 18 +++++++++---------
 src/backend/utils/init/postinit.c   |  4 ++--
 src/backend/utils/misc/guc_tables.c |  2 +-
 src/include/postmaster/postmaster.h |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 9cedc1b9f0..470704f364 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -204,15 +204,15 @@ char	   *Unix_socket_directories;
 char	   *ListenAddresses;
 
 /*
- * ReservedBackends is the number of backends reserved for superuser use.
- * This number is taken out of the pool size given by MaxConnections so
+ * SuperuserReservedBackends is the number of backends reserved for superuser
+ * use.  This number is taken out of the pool size given by MaxConnections so
  * number of backend slots available to non-superusers is
- * (MaxConnections - ReservedBackends).  Note what this really means is
- * "if there are <= ReservedBackends connections available, only superusers
- * can make new connections" --- pre-existing superuser connections don't
- * count against the limit.
+ * (MaxConnections - SuperuserReservedBackends).  Note what this really means
+ * is "if there are <= SuperuserReservedBackends connections available, only
+ * superusers can make new connections" --- pre-existing superuser connections
+ * don't count against the limit.
  */
-int			ReservedBackends;
+int			SuperuserReservedBackends;
 
 /* The socket(s) we're listening to. */
 #define MAXLISTEN	64
@@ -908,11 +908,11 @@ PostmasterMain(int argc, char *argv[])
 	/*
 	 * Check for invalid combinations of GUC settings.
 	 */
-	if (ReservedBackends >= MaxConnections)
+	if (SuperuserReservedBackends >= MaxConnections)
 	{
 		write_stderr("%s: superuser_reserved_connections (%d) must be less than max_connections (%d)\n",
 					 progname,
-					 ReservedBackends, MaxConnections);
+					 SuperuserReservedBackends, MaxConnections);
 		ExitPostmaster(1);
 	}
 	if (XLogArchiveMode > ARCHIVE_MODE_OFF && wal_level == WAL_LEVEL_MINIMAL)
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 9145d96b38..531a80f8d5 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -927,8 +927,8 @@ InitPostgres(const char *in_dbname, Oid dboid,
 	 * limited by max_connections or superuser_reserved_connections.
 	 */
 	if (!am_superuser && !am_walsender &&
-		ReservedBackends > 0 &&
-		!HaveNFreeProcs(ReservedBackends))
+		SuperuserReservedBackends > 0 &&
+		!HaveNFreeProcs(SuperuserReservedBackends))
 		ereport(FATAL,
 				(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
 				 errmsg("remaining connection slots are reserved for superusers")));
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 5025e80f89..5aa2cda8f9 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2163,7 +2163,7 @@ struct config_int ConfigureNamesInt[] =
 			gettext_noop("Sets the number of connection slots reserved for superusers."),
 			NULL
 		},
-		&ReservedBackends,
+		&SuperuserReservedBackends,
 		3, 0, MAX_BACKENDS,
 		NULL, NULL, NULL
 	},
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 203177e1ff..168d85a3d1 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -15,7 +15,7 @@
 
 /* GUC options */
 extern PGDLLIMPORT bool EnableSSL;
-extern PGDLLIMPORT int ReservedBackends;
+extern PGDLLIMPORT int SuperuserReservedBackends;
 extern PGDLLIMPORT int PostPortNumber;
 extern PGDLLIMPORT int Unix_socket_permissions;
 extern PGDLLIMPORT char *Unix_socket_group;
-- 
2.25.1

>From 17bd6189138c49bc222614195cbf202274ea8e9b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandboss...@gmail.com>
Date: Tue, 17 Jan 2023 15:36:59 -0800
Subject: [PATCH v3 3/3] Introduce reserved_connections and
 pg_use_reserved_connections.

This provides a way to reserve connection slots for non-superusers.
superuser_reserved_connections remains as a final reserve in case
reserved_connections has been exhausted.
---
 doc/src/sgml/config.sgml                      | 39 ++++++++++++++++++-
 doc/src/sgml/user-manag.sgml                  |  5 +++
 src/backend/postmaster/postmaster.c           | 28 ++++++++-----
 src/backend/storage/lmgr/proc.c               | 16 +++++---
 src/backend/utils/init/postinit.c             | 27 +++++++++----
 src/backend/utils/misc/guc_tables.c           | 11 ++++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/catalog/pg_authid.dat             |  5 +++
 src/include/postmaster/postmaster.h           |  1 +
 src/include/storage/proc.h                    |  2 +-
 10 files changed, 110 insertions(+), 25 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e019a1aac9..626faa4d08 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -725,17 +725,52 @@ include_dir 'conf.d'
         number of active concurrent connections is at least
         <varname>max_connections</varname> minus
         <varname>superuser_reserved_connections</varname>, new
-        connections will be accepted only for superusers.
+        connections will be accepted only for superusers.  The connection slots
+        reserved by this parameter are intended as final reserve for emergency
+        use after the slots reserved by
+        <xref linkend="guc-reserved-connections"/> have been exhausted.
        </para>
 
        <para>
         The default value is three connections. The value must be less
-        than <varname>max_connections</varname>.
+        than <varname>max_connections</varname> minus
+        <varname>reserved_connections</varname>.
         This parameter can only be set at server start.
        </para>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-reserved-connections" xreflabel="reserved_connections">
+      <term><varname>reserved_connections</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>reserved_connections</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Determines the number of connection <quote>slots</quote> that are
+        reserved for connections by roles with privileges of the
+        <link linkend="predefined-roles-table"><literal>pg_used_reserved_connections</literal></link>
+        role.  Whenever the number of free connection slots is greater than
+        <xref linkend="guc-superuser-reserved-connections"/> but less than or
+        equal to the sum of <varname>superuser_reserved_connections</varname>
+        and <varname>reserved_connections</varname>, new connections will be
+        accepted only for superusers and roles with privileges of
+        <literal>pg_use_reserved_connections</literal>.  If
+        <varname>superuser_reserved_connections</varname> or fewer connection
+        slots are available, new connections will be accepted only for
+        superusers.
+       </para>
+
+       <para>
+        The default value is zero connections.  The value must be less than
+        <varname>max_connections</varname> minus
+        <varname>superuser_reserved_connections</varname>.  This parameter can
+        only be set at server start.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-unix-socket-directories" xreflabel="unix_socket_directories">
       <term><varname>unix_socket_directories</varname> (<type>string</type>)
       <indexterm>
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 71a2d8f298..d553b1adab 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -689,6 +689,11 @@ DROP ROLE doomed_role;
        and <link linkend="sql-lock"><command>LOCK TABLE</command></link> on all
        relations.</entry>
       </row>
+      <row>
+       <entry>pg_use_reserved_backends</entry>
+       <entry>Allow use of connection slots reserved via
+       <xref linkend="guc-reserved-connections"/>.</entry>
+      </row>
      </tbody>
     </tgroup>
    </table>
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 470704f364..ac69d3fd9b 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -205,14 +205,23 @@ char	   *ListenAddresses;
 
 /*
  * SuperuserReservedBackends is the number of backends reserved for superuser
- * use.  This number is taken out of the pool size given by MaxConnections so
- * number of backend slots available to non-superusers is
- * (MaxConnections - SuperuserReservedBackends).  Note what this really means
- * is "if there are <= SuperuserReservedBackends connections available, only
- * superusers can make new connections" --- pre-existing superuser connections
- * don't count against the limit.
+ * use, and ReservedBackends is the number of backends reserved for use by
+ * roles with privileges of the pg_use_reserved_backends predefined role.
+ * These are taken out of the pool of MaxConnections backend slots, so the
+ * number of backend slots available for roles that are neither superuser nor
+ * have privileges of pg_use_reserved_backends is
+ * (MaxConnections - SuperuserReservedBackends - ReservedBackends).
+ *
+ * If the number of remaining slots is less than or equal to
+ * SuperuserReservedBackends, only superusers can make new connections.  If the
+ * number of remaining slots is greater than SuperuserReservedBackends but less
+ * than or equal to (SuperuserReservedBackends + ReservedBackends), only
+ * superusers and roles with privileges of pg_use_reserved_backends can make
+ * new connections.  Note that pre-existing superuser and
+ * pg_use_reserved_backends connections don't count against the limits.
  */
 int			SuperuserReservedBackends;
+int			ReservedBackends;
 
 /* The socket(s) we're listening to. */
 #define MAXLISTEN	64
@@ -908,11 +917,12 @@ PostmasterMain(int argc, char *argv[])
 	/*
 	 * Check for invalid combinations of GUC settings.
 	 */
-	if (SuperuserReservedBackends >= MaxConnections)
+	if (SuperuserReservedBackends + ReservedBackends >= MaxConnections)
 	{
-		write_stderr("%s: superuser_reserved_connections (%d) must be less than max_connections (%d)\n",
+		write_stderr("%s: superuser_reserved_connections (%d) plus reserved_connections (%d) must be less than max_connections (%d)\n",
 					 progname,
-					 SuperuserReservedBackends, MaxConnections);
+					 SuperuserReservedBackends, ReservedBackends,
+					 MaxConnections);
 		ExitPostmaster(1);
 	}
 	if (XLogArchiveMode > ARCHIVE_MODE_OFF && wal_level == WAL_LEVEL_MINIMAL)
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index f8ac4edd6f..22b4278610 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -645,27 +645,33 @@ GetStartupBufferPinWaitBufId(void)
 }
 
 /*
- * Check whether there are at least N free PGPROC objects.
+ * Check whether there are at least N free PGPROC objects.  If false is
+ * returned, *nfree will be set to the number of free PGPROC objects.
+ * Otherwise, *nfree will be set to n.
  *
  * Note: this is designed on the assumption that N will generally be small.
  */
 bool
-HaveNFreeProcs(int n)
+HaveNFreeProcs(int n, int *nfree)
 {
 	dlist_iter	iter;
 
+	Assert(n > 0);
+	Assert(nfree);
+
 	SpinLockAcquire(ProcStructLock);
 
+	*nfree = 0;
 	dlist_foreach(iter, &ProcGlobal->freeProcs)
 	{
-		n--;
-		if (n == 0)
+		(*nfree)++;
+		if (*nfree == n)
 			break;
 	}
 
 	SpinLockRelease(ProcStructLock);
 
-	return (n <= 0);
+	return (*nfree == n);
 }
 
 /*
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 531a80f8d5..bd7ece259a 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -719,6 +719,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
 	bool		am_superuser;
 	char	   *fullpath;
 	char		dbname[NAMEDATALEN];
+	int			nfree = 0;
 
 	elog(DEBUG3, "InitPostgres");
 
@@ -922,16 +923,26 @@ InitPostgres(const char *in_dbname, Oid dboid,
 	}
 
 	/*
-	 * The last few connection slots are reserved for superusers.  Replication
-	 * connections are drawn from slots reserved with max_wal_senders and not
-	 * limited by max_connections or superuser_reserved_connections.
+	 * The last few connection slots are reserved for superusers and roles with
+	 * privileges of pg_use_reserved_connections.  Replication connections are
+	 * drawn from slots reserved with max_wal_senders and are not limited by
+	 * max_connections, superuser_reserved_connections, or
+	 * reserved_connections.
 	 */
 	if (!am_superuser && !am_walsender &&
-		SuperuserReservedBackends > 0 &&
-		!HaveNFreeProcs(SuperuserReservedBackends))
-		ereport(FATAL,
-				(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
-				 errmsg("remaining connection slots are reserved for superusers")));
+		(SuperuserReservedBackends + ReservedBackends) > 0 &&
+		!HaveNFreeProcs(SuperuserReservedBackends + ReservedBackends, &nfree))
+	{
+		if (nfree < SuperuserReservedBackends)
+			ereport(FATAL,
+					(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
+					 errmsg("remaining connection slots are reserved for superusers")));
+
+		if (!has_privs_of_role(GetUserId(), ROLE_PG_USE_RESERVED_CONNECTIONS))
+			ereport(FATAL,
+					(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
+					 errmsg("remaining connection slots are reserved for roles with privileges of pg_use_reserved_backends")));
+	}
 
 	/* Check replication permissions needed for walsender processes. */
 	if (am_walsender)
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 5aa2cda8f9..77b17aa7a6 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2168,6 +2168,17 @@ struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"reserved_connections", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+			gettext_noop("Sets the number of connection slots reserved for roles "
+						 "with privileges of pg_use_reserved_connections."),
+			NULL
+		},
+		&ReservedBackends,
+		0, 0, MAX_BACKENDS,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"min_dynamic_shared_memory", PGC_POSTMASTER, RESOURCES_MEM,
 			gettext_noop("Amount of dynamic shared memory reserved at startup."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 4cceda4162..f701312225 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -64,6 +64,7 @@
 #port = 5432				# (change requires restart)
 #max_connections = 100			# (change requires restart)
 #superuser_reserved_connections = 3	# (change requires restart)
+#reserved_connections = 0		# (change requires restart)
 #unix_socket_directories = '/tmp'	# comma-separated list of directories
 					# (change requires restart)
 #unix_socket_group = ''			# (change requires restart)
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index 2a2fee7d28..f2e5663c9f 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -89,5 +89,10 @@
   rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
   rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
   rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '4550', oid_symbol => 'ROLE_PG_USE_RESERVED_CONNECTIONS',
+  rolname => 'pg_use_reserved_connections', rolsuper => 'f', rolinherit => 't',
+  rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+  rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
+  rolpassword => '_null_', rolvaliduntil => '_null_' },
 
 ]
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 168d85a3d1..18ef5afa75 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -16,6 +16,7 @@
 /* GUC options */
 extern PGDLLIMPORT bool EnableSSL;
 extern PGDLLIMPORT int SuperuserReservedBackends;
+extern PGDLLIMPORT int ReservedBackends;
 extern PGDLLIMPORT int PostPortNumber;
 extern PGDLLIMPORT int Unix_socket_permissions;
 extern PGDLLIMPORT char *Unix_socket_group;
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index dd45b8ee9b..4258cd92c9 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -445,7 +445,7 @@ extern void InitAuxiliaryProcess(void);
 extern void SetStartupBufferPinWaitBufId(int bufid);
 extern int	GetStartupBufferPinWaitBufId(void);
 
-extern bool HaveNFreeProcs(int n);
+extern bool HaveNFreeProcs(int n, int *nfree);
 extern void ProcReleaseLocks(bool isCommit);
 
 extern ProcWaitStatus ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable);
-- 
2.25.1

Reply via email to