From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: almost-super-user problems that we haven't fixed yet
Date: Wed, 18 Jan 2023 13:14:14 -0800
Message-ID: <20230118211414.GA3702050@nathanxps13> (raw)
In-Reply-To: <CA+Tgmob0SMMZhDEtRTsqOFP3dbGQoqBs=O2x6bp_UVeH1RVuVw@mail.gmail.com>
References: <CA+TgmoYwTFTQR0+qe+_asEvv2wqQL+0JCyexgVy2Ct31+rCb7w@mail.gmail.com>
<20230116223728.GA2714038@nathanxps13>
<CA+TgmoYmO+D3yB_eCpDewGLOcWMbZnKO7Rg2uh60bcmdJiXVHQ@mail.gmail.com>
<20230117184230.GB3015764@nathanxps13>
<CA+TgmoaOXE4rDzenFuaEFKy7DLN-3BRuP1WAetNEuswFc3yuvA@mail.gmail.com>
<20230118001523.GB3321806@nathanxps13>
<CA+Tgmoa3qkqVpU2=riFfKchOLDMuaQcFXOF-a_B-RRJm1Njq_w@mail.gmail.com>
<20230118190012.GA3540486@nathanxps13>
<CA+Tgmob0SMMZhDEtRTsqOFP3dbGQoqBs=O2x6bp_UVeH1RVuVw@mail.gmail.com>
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.comAttachments:
[text/x-diff] v3-0001-Code-review-for-ea92368.patch (1.6K, ../20230118211414.GA3702050@nathanxps13/2-v3-0001-Code-review-for-ea92368.patch)
download | inline diff:
From 8f0aa2fa54ae01149cffe9a69265f98e76a08a23 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@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.sgmlindex 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.cindex 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
[text/x-diff] v3-0002-Rename-ReservedBackends-to-SuperuserReservedBacke.patch (4.0K, ../20230118211414.GA3702050@nathanxps13/3-v3-0002-Rename-ReservedBackends-to-SuperuserReservedBacke.patch)
download | inline diff:
From bc110461e4f0a73c2b76ba0a6e821349c2cbe3df Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@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.cindex 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.cindex 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.cindex 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.hindex 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
[text/x-diff] v3-0003-Introduce-reserved_connections-and-pg_use_reserve.patch (12.8K, ../20230118211414.GA3702050@nathanxps13/4-v3-0003-Introduce-reserved_connections-and-pg_use_reserve.patch)
download | inline diff:
From 17bd6189138c49bc222614195cbf202274ea8e9b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@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.sgmlindex 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.sgmlindex 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.cindex 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.cindex 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.cindex 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.cindex 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.sampleindex 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.datindex 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.hindex 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.hindex 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 instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, robertmhaas@gmail.com
Subject: Re: almost-super-user problems that we haven't fixed yet
In-Reply-To: <20230118211414.GA3702050@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox