pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedalmost-super-user problems that we haven't fixed yet
24+ messages / 3 participants
[nested] [flat]
* almost-super-user problems that we haven't fixed yet
@ 2023-01-16 19:29 Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Robert Haas @ 2023-01-16 19:29 UTC (permalink / raw)
To: pgsql-hackers
Due to cf5eb37c5ee0cc54c80d95c1695d7fca1f7c68cb,
e5b8a4c098ad6add39626a14475148872cd687e0, and prior commits touching
related code, it should now be possible to consider handing out
CREATEROLE as a reasonable alternative to handing out SUPERUSER. Prior
to cf5eb37c5ee0cc54c80d95c1695d7fca1f7c68cb, giving CREATEROLE meant
giving away control of pg_execute_server_programs and every other
built-in role, so it wasn't possible to give CREATEROLE to a user who
isn't completely trusted. Now, that should be OK. CREATEROLE users
will only gain control over roles they create (and any others that the
superuser grants to them). Furthermore, if you set
createrole_self_grant to 'inherit' or 'set, inherit', a CREATEROLE
user will automatically inherit the privileges of the users they
create, hopefully making them feel like they are almost a superuser
without letting them actually take over the world.
Not very surprisingly, those commits failed to solve every single
problem that anyone has ever thought about in this area.
Here is a probably-incomplete list of related problems that are so far unsolved:
1. It's still possible for a CREATEROLE user to hand out role
attributes that they don't possess. The new prohibitions in
cf5eb37c5ee0cc54c80d95c1695d7fca1f7c68cb prevent a CREATEROLE user
from handing out membership in a role on which they lack sufficient
permissions, but they don't prevent a CREATEROLE user who lacks
CREATEDB from creating a new user who does have CREATEDB. I think we
should subject the CREATEDB, REPLICATION, and BYPASSRLS attributes to
the same rule that we now use for role memberships: you've got to have
the property in order to give it to someone else. In the case of
CREATEDB, this would tighten the current rules, which allow you to
give out CREATEDB without having it. In the case of REPLICATION and
BYPASSRLS, this would liberalize the current rules: right now, a
CREATEROLE user cannot give REPLICATION or BYPASSRLS to another user
even if they possess those attributes.
This proposal doesn't address the CREATEROLE or CONNECTION LIMIT
properties. It seems possible to me that someone might want to set up
a CREATEROLE user who can't make more such users, and this proposal
doesn't manufacture any way of doing that. It also doesn't let you
constraint the ability of a CREATEROLE user to set a CONNECTION LIMIT
for some other user. I think that's OK. It might be nice to have ways
of imposing such restrictions at some point in the future, but it is
not very obvious what to do about such cases and, importantly, I don't
think there's any security impact from failing to address those cases.
If a CREATEROLE user without CREATEDB can create a new role that does
have CREATEDB, that's a privilege escalation. If they can hand out
CREATEROLE, that isn't: they already have it.
2. It's still impossible for a CREATEROLE user to execute CREATE
SUBSCRIPTION, so they can't get logical replication working. There was
a previous thread about fixing this at
https://www.postgresql.org/message-id/flat/9DFC88D3-1300-4DE8-ACBC-4CEF84399A53%40enterprisedb.com
and the corresponding CF entry is listed as committed, but
CreateSubscription() still requires superuser, so I think that maybe
that thread only got some of the preliminary permissions-check work
committed and the core problem is yet to be solved.
3. Only superusers can control event triggers. In the thread at
https://www.postgresql.org/message-id/flat/914FF898-5AC4-4E02-8A05-3876087007FB%40enterprisedb.com
it was proposed, based on an idea from Tom, to allow any user to
create event triggers but, approximately, to only have them fire for
code running as a user whose privileges the creator already has. I
don't recall the precise rule that was proposed and it might need
rethinking in view of 3d14e171e9e2236139e8976f3309a588bcc8683b, and I
think there was also some opposition to that proposal, so I'm not sure
what the way forward here is.
4. You can reserve a small number of connections for the superuser
with superuser_reserved_connections, but there's no way to do a
similar thing for any other user. As mentioned above, a CREATEROLE
user could set connection limits for every created role such that the
sum of those limits is less than max_connections by some margin, but
that restricts each of those roles individually, not all of them in
the aggregate. Maybe we could address this by inventing a new GUC
reserved_connections and a predefined role
pg_use_reserved_connections.
5. If you set createrole_self_grant = 'set, inherit' and make alice a
CREATEROLE user and she goes around and creates a bunch of other users
and they all run around and create a bunch of objects and then alice
tries to pg_dump the entire database, it will work ... provided that
there are no tables owned by any other user. If the superuser has
created any tables, or there's another CREATEROLE user wandering
around creating tables, or even a non-CREATEROLE user whose
permissions alice does not have, pg_dump will try to lock them and
die. I don't see any perfect solution to this problem: we can neither
let alice dump objects on which she does not have permission, nor can
we silently skip them in the interest of giving alice a better user
experience, because if we do that then somebody will end up with a
partial database backup that they think is a complete database backup
and that will be a really bad day. However, I think we could add a
pg_dump option that says, hey, please only try to dump tables we have
permission to dump, and skip the others. Or, of course, alice could
use -T and -N as required, but a dedicated switch for
skip-stuff-i-can't-access-quietly might be a better user experience. I
guess you could also argue that this isn't really a problem in the
first place because you could always choose to grant
pg_read_all_tables to the almost-super-user, but maybe that's not
always desirable. Not sure.
Just to be clear, there are lots of other things that a non-superuser
cannot do, such as CREATE LANGUAGE. However, I'm excluding that kind
of thing from this list because it's intrinsically unsafe to allow a
non-superuser to do that, since it's probably a gateway to arbitrary
code execution and then you can probably get superuser for real, and
control of the OS account, too. What I'm interested in is developing a
list of things that could, with the right infrastructure, be delegated
to non-superusers safely but which, as things stand today, cannot be
delegated to non-superusers. Contributions to the list are most
welcome as are thoughts on the proposals above.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-16 22:37 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-01-16 22:37 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On Mon, Jan 16, 2023 at 02:29:56PM -0500, Robert Haas wrote:
> 4. You can reserve a small number of connections for the superuser
> with superuser_reserved_connections, but there's no way to do a
> similar thing for any other user. As mentioned above, a CREATEROLE
> user could set connection limits for every created role such that the
> sum of those limits is less than max_connections by some margin, but
> that restricts each of those roles individually, not all of them in
> the aggregate. Maybe we could address this by inventing a new GUC
> reserved_connections and a predefined role
> pg_use_reserved_connections.
I've written something like this before, and I'd be happy to put together a
patch if there is interest.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-17 02:06 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Robert Haas @ 2023-01-17 02:06 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
On Mon, Jan 16, 2023 at 5:37 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> On Mon, Jan 16, 2023 at 02:29:56PM -0500, Robert Haas wrote:
> > 4. You can reserve a small number of connections for the superuser
> > with superuser_reserved_connections, but there's no way to do a
> > similar thing for any other user. As mentioned above, a CREATEROLE
> > user could set connection limits for every created role such that the
> > sum of those limits is less than max_connections by some margin, but
> > that restricts each of those roles individually, not all of them in
> > the aggregate. Maybe we could address this by inventing a new GUC
> > reserved_connections and a predefined role
> > pg_use_reserved_connections.
>
> I've written something like this before, and I'd be happy to put together a
> patch if there is interest.
Cool. I had been thinking of coding it up myself, but you doing it works, too.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-17 18:42 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-01-17 18:42 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On Mon, Jan 16, 2023 at 09:06:10PM -0500, Robert Haas wrote:
> On Mon, Jan 16, 2023 at 5:37 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> On Mon, Jan 16, 2023 at 02:29:56PM -0500, Robert Haas wrote:
>> > 4. You can reserve a small number of connections for the superuser
>> > with superuser_reserved_connections, but there's no way to do a
>> > similar thing for any other user. As mentioned above, a CREATEROLE
>> > user could set connection limits for every created role such that the
>> > sum of those limits is less than max_connections by some margin, but
>> > that restricts each of those roles individually, not all of them in
>> > the aggregate. Maybe we could address this by inventing a new GUC
>> > reserved_connections and a predefined role
>> > pg_use_reserved_connections.
>>
>> I've written something like this before, and I'd be happy to put together a
>> patch if there is interest.
>
> Cool. I had been thinking of coding it up myself, but you doing it works, too.
Alright. The one design question I have is whether this should be a new
set of reserved connections or replace superuser_reserved_connections
entirely.
If we create a new batch of reserved connections, only roles with
privileges of pg_use_reserved_connections would be able to connect if the
number of remaining slots is greater than superuser_reserved_connections
but less than or equal to superuser_reserved_connections +
reserved_connections. Only superusers would be able to connect if the
number of remaining slots is less than or equal to
superuser_reserved_connections. This helps avoid blocking new superuser
connections even if you've reserved some connections for non-superusers.
Іf we replace superuser_reserved_connections, we're basically opening up
the existing functionality to non-superusers, which is simpler and probably
more in the spirit of this thread, but it doesn't provide a way to prevent
blocking new superuser connections.
My preference is the former approach. This is closest to what I've written
before, and if I read your words carefully, it seems to be what you are
proposing. WDYT?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-17 19:59 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Robert Haas @ 2023-01-17 19:59 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
On Tue, Jan 17, 2023 at 1:42 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> Alright. The one design question I have is whether this should be a new
> set of reserved connections or replace superuser_reserved_connections
> entirely.
I think it should definitely be something new, not a replacement.
> If we create a new batch of reserved connections, only roles with
> privileges of pg_use_reserved_connections would be able to connect if the
> number of remaining slots is greater than superuser_reserved_connections
> but less than or equal to superuser_reserved_connections +
> reserved_connections. Only superusers would be able to connect if the
> number of remaining slots is less than or equal to
> superuser_reserved_connections. This helps avoid blocking new superuser
> connections even if you've reserved some connections for non-superusers.
This is precisely what I had in mind.
I think the documentation will need some careful wordsmithing,
including adjustments to superuser_reserved_connections. We want to
recast superuser_reserved_connections as a final reserve to be touched
after even reserved_connections has been exhausted.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-18 00:15 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-01-18 00:15 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On Tue, Jan 17, 2023 at 02:59:31PM -0500, Robert Haas wrote:
> On Tue, Jan 17, 2023 at 1:42 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> If we create a new batch of reserved connections, only roles with
>> privileges of pg_use_reserved_connections would be able to connect if the
>> number of remaining slots is greater than superuser_reserved_connections
>> but less than or equal to superuser_reserved_connections +
>> reserved_connections. Only superusers would be able to connect if the
>> number of remaining slots is less than or equal to
>> superuser_reserved_connections. This helps avoid blocking new superuser
>> connections even if you've reserved some connections for non-superusers.
>
> This is precisely what I had in mind.
Great. Here is a first attempt at the patch.
> I think the documentation will need some careful wordsmithing,
> including adjustments to superuser_reserved_connections. We want to
> recast superuser_reserved_connections as a final reserve to be touched
> after even reserved_connections has been exhausted.
I tried to do this, but there is probably still room for improvement,
especially for the parts that discuss the relationship between
max_connections, superuser_reserved_connections, and reserved_connections.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v1-0001-Rename-ReservedBackends-to-SuperuserReservedBacke.patch (4.0K, ../../20230118001523.GB3321806@nathanxps13/2-v1-0001-Rename-ReservedBackends-to-SuperuserReservedBacke.patch)
download | inline diff:
From e153d2f22d4303d0bb5e8134391ebf1fa446172c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 17 Jan 2023 13:58:56 -0800
Subject: [PATCH v1 1/2] 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 ae5a85ed65..6fa696fe8d 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 non-replication superuser connections")));
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
[text/x-diff] v1-0002-Introduce-reserved_connections-and-pg_use_reserve.patch (10.9K, ../../20230118001523.GB3321806@nathanxps13/3-v1-0002-Introduce-reserved_connections-and-pg_use_reserve.patch)
download | inline diff:
From 2961ebbeb1e2910752fa4297f6aaa52492b30741 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 17 Jan 2023 15:36:59 -0800
Subject: [PATCH v1 2/2] 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 | 40 +++++++++++++++++--
doc/src/sgml/user-manag.sgml | 5 +++
src/backend/postmaster/postmaster.c | 28 ++++++++-----
src/backend/utils/init/postinit.c | 16 ++++++--
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 +
8 files changed, 92 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 89d53f2a64..aa100870b9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -725,18 +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, and no
- new replication connections will be accepted.
+ 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 three 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/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 6fa696fe8d..4e74cdbb9e 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -922,9 +922,11 @@ 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 &&
@@ -933,6 +935,14 @@ InitPostgres(const char *in_dbname, Oid dboid,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for non-replication superuser connections")));
+ if (!has_privs_of_role(GetUserId(), ROLE_PG_USE_RESERVED_CONNECTIONS) &&
+ !am_walsender &&
+ ReservedBackends > 0 &&
+ !HaveNFreeProcs(SuperuserReservedBackends + ReservedBackends))
+ 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..1e94e70b42 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,
+ 3, 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..5107afe140 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 = 3 # (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;
--
2.25.1
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-18 16:28 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Robert Haas @ 2023-01-18 16:28 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
On Tue, Jan 17, 2023 at 7:15 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> Great. Here is a first attempt at the patch.
In general, looks good. I think this will often call HaveNFreeProcs
twice, though, and that would be better to avoid, e.g.
if (!am_superuser && !am_walsender && (SuperuserReservedBackends +
ReservedBackends) > 0)
&& !HaveNFreeProcs(SuperuserReservedBackends + ReservedBackends))
{
if (!HaveNFreeProcs(SuperuserReservedBackends))
remaining connection slots are reserved for non-replication
superuser connections;
if (!has_privs_of_role(GetUserId(), ROLE_PG_USE_RESERVED_CONNECTIONS))
remaining connection slots are reserved for roles with
privileges of pg_use_reserved_backends;
}
In the common case where we hit neither limit, this only counts free
connection slots once. We could do even better by making
HaveNFreeProcs have an out parameter for the number of free procs
actually found when it returns false, but that's probably not
important.
I don't think that we should default both the existing GUC and the new
one to 3, because that raises the default limit in the case where the
new feature is not used from 3 to 6. I think we should default one of
them to 0 and the other one to 3. Not sure which one should get which
value.
> > I think the documentation will need some careful wordsmithing,
> > including adjustments to superuser_reserved_connections. We want to
> > recast superuser_reserved_connections as a final reserve to be touched
> > after even reserved_connections has been exhausted.
>
> I tried to do this, but there is probably still room for improvement,
> especially for the parts that discuss the relationship between
> max_connections, superuser_reserved_connections, and reserved_connections.
I think it's pretty good the way you have it. I agree that there might
be a way to make it even better, but I don't think I know what it is.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-18 19:00 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-01-18 19:00 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On Wed, Jan 18, 2023 at 11:28:57AM -0500, Robert Haas wrote:
> In general, looks good. I think this will often call HaveNFreeProcs
> twice, though, and that would be better to avoid, e.g.
I should have thought of this. This is fixed in v2.
> In the common case where we hit neither limit, this only counts free
> connection slots once. We could do even better by making
> HaveNFreeProcs have an out parameter for the number of free procs
> actually found when it returns false, but that's probably not
> important.
Actually, I think it might be important. IIUC the separate calls to
HaveNFreeProcs might return different values for the same input, which
could result in incorrect error messages (e.g., you might get the
reserved_connections message despite setting reserved_connections to 0).
So, I made this change in v2, too.
> I don't think that we should default both the existing GUC and the new
> one to 3, because that raises the default limit in the case where the
> new feature is not used from 3 to 6. I think we should default one of
> them to 0 and the other one to 3. Not sure which one should get which
> value.
I chose to set reserved_connections to 0 since it is new and doesn't have a
pre-existing default value.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v2-0001-Rename-ReservedBackends-to-SuperuserReservedBacke.patch (4.0K, ../../20230118190012.GA3540486@nathanxps13/2-v2-0001-Rename-ReservedBackends-to-SuperuserReservedBacke.patch)
download | inline diff:
From 94ee89548fd1080447b784993a1418480f407b49 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 17 Jan 2023 13:58:56 -0800
Subject: [PATCH v2 1/2] 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 ae5a85ed65..6fa696fe8d 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 non-replication superuser connections")));
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
[text/x-diff] v2-0002-Introduce-reserved_connections-and-pg_use_reserve.patch (12.9K, ../../20230118190012.GA3540486@nathanxps13/3-v2-0002-Introduce-reserved_connections-and-pg_use_reserve.patch)
download | inline diff:
From ba7e566a097fd5ffbc20f67a9caccea31ef19212 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 17 Jan 2023 15:36:59 -0800
Subject: [PATCH v2 2/2] 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 | 40 +++++++++++++++++--
doc/src/sgml/user-manag.sgml | 5 +++
src/backend/postmaster/postmaster.c | 28 ++++++++-----
src/backend/storage/lmgr/proc.c | 13 +++---
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, 107 insertions(+), 26 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 89d53f2a64..626faa4d08 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -725,18 +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, and no
- new replication connections will be accepted.
+ 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 00d26dc0f6..bcd5363511 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -651,12 +651,14 @@ 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)
{
PGPROC *proc;
@@ -664,15 +666,16 @@ HaveNFreeProcs(int n)
proc = ProcGlobal->freeProcs;
- while (n > 0 && proc != NULL)
+ *nfree = 0;
+ while (*nfree < n && proc != NULL)
{
proc = (PGPROC *) proc->links.next;
- n--;
+ (*nfree)++;
}
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 6fa696fe8d..2a007f633b 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 non-replication superuser connections")));
+ (SuperuserReservedBackends + ReservedBackends) > 0 &&
+ !HaveNFreeProcs(SuperuserReservedBackends + ReservedBackends, &nfree))
+ {
+ if (nfree < SuperuserReservedBackends)
+ ereport(FATAL,
+ (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
+ errmsg("remaining connection slots are reserved for non-replication superuser connections")));
+
+ 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 b5c6f46d03..5221014737 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 void ProcQueueInit(PROC_QUEUE *queue);
--
2.25.1
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-18 19:51 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Robert Haas @ 2023-01-18 19:51 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
On Wed, Jan 18, 2023 at 2:00 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> On Wed, Jan 18, 2023 at 11:28:57AM -0500, Robert Haas wrote:
> > In general, looks good. I think this will often call HaveNFreeProcs
> > twice, though, and that would be better to avoid, e.g.
>
> I should have thought of this. This is fixed in v2.
Should (nfree < SuperuserReservedBackends) be using <=, or am I confused?
> > In the common case where we hit neither limit, this only counts free
> > connection slots once. We could do even better by making
> > HaveNFreeProcs have an out parameter for the number of free procs
> > actually found when it returns false, but that's probably not
> > important.
>
> Actually, I think it might be important. IIUC the separate calls to
> HaveNFreeProcs might return different values for the same input, which
> could result in incorrect error messages (e.g., you might get the
> reserved_connections message despite setting reserved_connections to 0).
> So, I made this change in v2, too.
I thought of that briefly and it didn't seem that important, but the
way you did it seems fine, so let's go with that.
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?
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-18 21:14 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 2 replies; 24+ messages in thread
From: Nathan Bossart @ 2023-01-18 21:14 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
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
Attachments:
[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.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
[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.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
[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.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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 12:58 tushar <tushar.ahuja@enterprisedb.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 2 replies; 24+ messages in thread
From: tushar @ 2023-01-19 12:58 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On 1/19/23 2:44 AM, Nathan Bossart wrote:
> 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.
>
There is one typo , for the doc changes, it is mentioned
"pg_use_reserved_backends" but i think it supposed to be
"pg_use_reserved_connections"
under Table 22.1. Predefined Roles.
--
regards,tushar
EnterpriseDB https://www.enterprisedb.com/
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 13:20 tushar <tushar.ahuja@enterprisedb.com>
parent: tushar <tushar.ahuja@enterprisedb.com>
1 sibling, 1 reply; 24+ messages in thread
From: tushar @ 2023-01-19 13:20 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On Thu, Jan 19, 2023 at 6:28 PM tushar <tushar.ahuja@enterprisedb.com>
wrote:
> On 1/19/23 2:44 AM, Nathan Bossart wrote:
> > 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.
> >
> There is one typo , for the doc changes, it is mentioned
> "pg_use_reserved_backends" but i think it supposed to be
> "pg_use_reserved_connections"
> under Table 22.1. Predefined Roles.
>
> and in the error message too
[edb@centos7tushar bin]$ ./psql postgres -U r2
psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed:
FATAL: remaining connection slots are reserved for roles with privileges
of pg_use_reserved_backends
[edb@centos7tushar bin]$
regards,
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 14:21 tushar <tushar.ahuja@enterprisedb.com>
parent: tushar <tushar.ahuja@enterprisedb.com>
0 siblings, 1 reply; 24+ messages in thread
From: tushar @ 2023-01-19 14:21 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On Thu, Jan 19, 2023 at 6:50 PM tushar <tushar.ahuja@enterprisedb.com>
wrote:
> and in the error message too
>
> [edb@centos7tushar bin]$ ./psql postgres -U r2
>
> psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed:
> FATAL: remaining connection slots are reserved for roles with privileges
> of pg_use_reserved_backends
> [edb@centos7tushar bin]$
>
I think there is also a need to improve the error message if non
super users are not able to connect due to slot unavailability.
--Connect to psql terminal, create a user
create user t1;
--set these GUC parameters in postgresql.conf and restart the server
max_connections = 3 # (change requires restart)
superuser_reserved_connections = 1 # (change requires restart)
reserved_connections = 1
psql terminal ( connect to superuser), ./psql postgres
psql terminal (try to connect to user t1) , ./psql postgres -U t1
Error message is
psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed:
FATAL: remaining connection slots are reserved for roles with privileges
of pg_use_reserved_backends
that is not true because the superuser can still able to connect,
probably in this case message should be like this -
"remaining connection slots are reserved for roles with privileges of
pg_use_reserved_connections and for superusers" or something better.
regards,
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 15:31 Robert Haas <robertmhaas@gmail.com>
parent: tushar <tushar.ahuja@enterprisedb.com>
0 siblings, 0 replies; 24+ messages in thread
From: Robert Haas @ 2023-01-19 15:31 UTC (permalink / raw)
To: tushar <tushar.ahuja@enterprisedb.com>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On Thu, Jan 19, 2023 at 9:21 AM tushar <tushar.ahuja@enterprisedb.com> wrote:
> that is not true because the superuser can still able to connect,
It is true, but because superusers have all privileges.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 16:40 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 1 reply; 24+ messages in thread
From: Robert Haas @ 2023-01-19 16:40 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
On Wed, Jan 18, 2023 at 4:14 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> 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.
OK. Might be worth a short comment.
> > 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.
I'm inclined to commit it to master and not back-patch. It doesn't
seem important enough to perturb translations.
Tushar seems to have a point about pg_use_reserved_connections vs.
pg_use_reserved_backends. I think we should standardize on the former,
as backends is an internal term.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 17:54 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-01-19 17:54 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On Thu, Jan 19, 2023 at 11:40:53AM -0500, Robert Haas wrote:
> On Wed, Jan 18, 2023 at 4:14 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> 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.
>
> OK. Might be worth a short comment.
I added one.
>> > 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.
>
> I'm inclined to commit it to master and not back-patch. It doesn't
> seem important enough to perturb translations.
That seems reasonable to me.
> Tushar seems to have a point about pg_use_reserved_connections vs.
> pg_use_reserved_backends. I think we should standardize on the former,
> as backends is an internal term.
Oops. This is what I meant to do. I probably flubbed it because I was
wondering why the parameter uses "connections" and the variable uses
"backends," especially considering that the variable for max_connections is
called MaxConnections. I went ahead and renamed everything to use
"connections."
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v4-0001-Code-review-for-ea92368.patch (1.6K, ../../20230119175421.GA3934756@nathanxps13/2-v4-0001-Code-review-for-ea92368.patch)
download | inline diff:
From 7be7e70aaf488a924d61b21b351a3b4f7e33aedc Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Wed, 18 Jan 2023 12:43:41 -0800
Subject: [PATCH v4 1/3] Code review for ea92368.
This commit missed an error message and a line in the docs.
---
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
[text/x-diff] v4-0002-Rename-ReservedBackends-to-SuperuserReservedConne.patch (4.1K, ../../20230119175421.GA3934756@nathanxps13/3-v4-0002-Rename-ReservedBackends-to-SuperuserReservedConne.patch)
download | inline diff:
From 058df2b3dcf50ecbe76c794f4f52751e6a9f765f Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 17 Jan 2023 13:58:56 -0800
Subject: [PATCH v4 2/3] Rename ReservedBackends to
SuperuserReservedConnections.
This is in preparation for adding a new reserved_connections GUC.
---
src/backend/postmaster/postmaster.c | 20 ++++++++++----------
src/backend/utils/init/postinit.c | 4 ++--
src/backend/utils/misc/guc_tables.c | 2 +-
src/include/postmaster/postmaster.h | 2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 9cedc1b9f0..3f799c4ac8 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
- * 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.
+ * SuperuserReservedConnections 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 - SuperuserReservedConnections). Note what this really
+ * means is "if there are <= SuperuserReservedConnections connections
+ * available, only superusers can make new connections" --- pre-existing
+ * superuser connections don't count against the limit.
*/
-int ReservedBackends;
+int SuperuserReservedConnections;
/* 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 (SuperuserReservedConnections >= MaxConnections)
{
write_stderr("%s: superuser_reserved_connections (%d) must be less than max_connections (%d)\n",
progname,
- ReservedBackends, MaxConnections);
+ SuperuserReservedConnections, 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..40f145e0ab 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))
+ SuperuserReservedConnections > 0 &&
+ !HaveNFreeProcs(SuperuserReservedConnections))
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 cd0fc2cb8f..0fa9fdd3c5 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,
+ &SuperuserReservedConnections,
3, 0, MAX_BACKENDS,
NULL, NULL, NULL
},
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 203177e1ff..0e4b8ded34 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 SuperuserReservedConnections;
extern PGDLLIMPORT int PostPortNumber;
extern PGDLLIMPORT int Unix_socket_permissions;
extern PGDLLIMPORT char *Unix_socket_group;
--
2.25.1
[text/x-diff] v4-0003-Introduce-reserved_connections-and-pg_use_reserve.patch (13.1K, ../../20230119175421.GA3934756@nathanxps13/4-v4-0003-Introduce-reserved_connections-and-pg_use_reserve.patch)
download | inline diff:
From 3c137c30d287ea7388d77ee7f3a5757227638c51 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 17 Jan 2023 15:36:59 -0800
Subject: [PATCH v4 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 | 29 +++++++++-----
src/backend/storage/lmgr/proc.c | 16 +++++---
src/backend/utils/init/postinit.c | 31 +++++++++++----
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, 115 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..002c1e3aff 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_connections</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 3f799c4ac8..aca1ef91b5 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -205,14 +205,24 @@ char *ListenAddresses;
/*
* SuperuserReservedConnections 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 - SuperuserReservedConnections). Note what this really
- * means is "if there are <= SuperuserReservedConnections connections
- * available, only superusers can make new connections" --- pre-existing
- * superuser connections don't count against the limit.
+ * superuser use, and ReservedConnections is the number of backends reserved
+ * for use by roles with privileges of the pg_use_reserved_connections
+ * 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_connections is
+ * (MaxConnections - SuperuserReservedConnections - ReservedConnections).
+ *
+ * If the number of remaining slots is less than or equal to
+ * SuperuserReservedConnections, only superusers can make new connections. If
+ * the number of remaining slots is greater than SuperuserReservedConnections
+ * but less than or equal to
+ * (SuperuserReservedConnections + ReservedConnections), only superusers and
+ * roles with privileges of pg_use_reserved_connections can make new
+ * connections. Note that pre-existing superuser and
+ * pg_use_reserved_connections connections don't count against the limits.
*/
int SuperuserReservedConnections;
+int ReservedConnections;
/* The socket(s) we're listening to. */
#define MAXLISTEN 64
@@ -908,11 +918,12 @@ PostmasterMain(int argc, char *argv[])
/*
* Check for invalid combinations of GUC settings.
*/
- if (SuperuserReservedConnections >= MaxConnections)
+ if (SuperuserReservedConnections + ReservedConnections >= 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,
- SuperuserReservedConnections, MaxConnections);
+ SuperuserReservedConnections, ReservedConnections,
+ 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 40f145e0ab..2f07ca7a0e 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,30 @@ 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.
+ *
+ * Note: At this point, the new backend has already claimed a proc struct,
+ * so we must check whether the number of free slots is strictly less than
+ * the reserved connection limits.
*/
if (!am_superuser && !am_walsender &&
- SuperuserReservedConnections > 0 &&
- !HaveNFreeProcs(SuperuserReservedConnections))
- ereport(FATAL,
- (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
- errmsg("remaining connection slots are reserved for superusers")));
+ (SuperuserReservedConnections + ReservedConnections) > 0 &&
+ !HaveNFreeProcs(SuperuserReservedConnections + ReservedConnections, &nfree))
+ {
+ if (nfree < SuperuserReservedConnections)
+ 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_connections")));
+ }
/* 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 0fa9fdd3c5..e1753a40fa 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
+ },
+ &ReservedConnections,
+ 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 0e4b8ded34..3b3889c58c 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 SuperuserReservedConnections;
+extern PGDLLIMPORT int ReservedConnections;
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 19:17 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Robert Haas @ 2023-01-19 19:17 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
On Thu, Jan 19, 2023 at 12:54 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:
> > OK. Might be worth a short comment.
>
> I added one.
Thanks. I'd move it to the inner indentation level so it's closer to
the test at issue.
I would also suggest reordering the documentation and the
postgresql.conf.sample file so that reserved_connections precedes
superuser_reserved_connections, instead of following it.
Other than that, this seems like it might be about ready to commit,
barring objections or bug reports.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 19:46 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-01-19 19:46 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On Thu, Jan 19, 2023 at 02:17:35PM -0500, Robert Haas wrote:
> On Thu, Jan 19, 2023 at 12:54 PM Nathan Bossart
> <nathandbossart@gmail.com> wrote:
>> > OK. Might be worth a short comment.
>>
>> I added one.
>
> Thanks. I'd move it to the inner indentation level so it's closer to
> the test at issue.
I meant for it to cover the call to HaveNFreeProcs() as well since the same
idea applies. I left it the same for now, but if you still think it makes
sense to move it, I'll do so.
> I would also suggest reordering the documentation and the
> postgresql.conf.sample file so that reserved_connections precedes
> superuser_reserved_connections, instead of following it.
Makes sense.
> Other than that, this seems like it might be about ready to commit,
> barring objections or bug reports.
Awesome.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v5-0001-Code-review-for-ea92368.patch (1.6K, ../../20230119194601.GA4105788@nathanxps13/2-v5-0001-Code-review-for-ea92368.patch)
download | inline diff:
From a6811b643df94c9057373fd687398c85a807fd5e Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Wed, 18 Jan 2023 12:43:41 -0800
Subject: [PATCH v5 1/3] Code review for ea92368.
This commit missed an error message and a line in the docs.
---
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
[text/x-diff] v5-0002-Rename-ReservedBackends-to-SuperuserReservedConne.patch (4.1K, ../../20230119194601.GA4105788@nathanxps13/3-v5-0002-Rename-ReservedBackends-to-SuperuserReservedConne.patch)
download | inline diff:
From e0390f0120315746ea04a9fa1bf709def76e6196 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 17 Jan 2023 13:58:56 -0800
Subject: [PATCH v5 2/3] Rename ReservedBackends to
SuperuserReservedConnections.
This is in preparation for adding a new reserved_connections GUC.
---
src/backend/postmaster/postmaster.c | 20 ++++++++++----------
src/backend/utils/init/postinit.c | 4 ++--
src/backend/utils/misc/guc_tables.c | 2 +-
src/include/postmaster/postmaster.h | 2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 9cedc1b9f0..3f799c4ac8 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
- * 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.
+ * SuperuserReservedConnections 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 - SuperuserReservedConnections). Note what this really
+ * means is "if there are <= SuperuserReservedConnections connections
+ * available, only superusers can make new connections" --- pre-existing
+ * superuser connections don't count against the limit.
*/
-int ReservedBackends;
+int SuperuserReservedConnections;
/* 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 (SuperuserReservedConnections >= MaxConnections)
{
write_stderr("%s: superuser_reserved_connections (%d) must be less than max_connections (%d)\n",
progname,
- ReservedBackends, MaxConnections);
+ SuperuserReservedConnections, 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..40f145e0ab 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))
+ SuperuserReservedConnections > 0 &&
+ !HaveNFreeProcs(SuperuserReservedConnections))
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 cd0fc2cb8f..0fa9fdd3c5 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,
+ &SuperuserReservedConnections,
3, 0, MAX_BACKENDS,
NULL, NULL, NULL
},
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 203177e1ff..0e4b8ded34 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 SuperuserReservedConnections;
extern PGDLLIMPORT int PostPortNumber;
extern PGDLLIMPORT int Unix_socket_permissions;
extern PGDLLIMPORT char *Unix_socket_group;
--
2.25.1
[text/x-diff] v5-0003-Introduce-reserved_connections-and-pg_use_reserve.patch (13.2K, ../../20230119194601.GA4105788@nathanxps13/4-v5-0003-Introduce-reserved_connections-and-pg_use_reserve.patch)
download | inline diff:
From 107d753e1eead744edd960b255bcf1b29ef41514 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 17 Jan 2023 15:36:59 -0800
Subject: [PATCH v5 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 | 29 +++++++++-----
src/backend/storage/lmgr/proc.c | 16 +++++---
src/backend/utils/init/postinit.c | 31 +++++++++++----
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, 115 insertions(+), 25 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e019a1aac9..dc9b78b0b7 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -708,6 +708,37 @@ include_dir 'conf.d'
</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-superuser-reserved-connections"
xreflabel="superuser_reserved_connections">
<term><varname>superuser_reserved_connections</varname>
@@ -725,12 +756,16 @@ 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>
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 71a2d8f298..002c1e3aff 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_connections</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 3f799c4ac8..aca1ef91b5 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -205,14 +205,24 @@ char *ListenAddresses;
/*
* SuperuserReservedConnections 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 - SuperuserReservedConnections). Note what this really
- * means is "if there are <= SuperuserReservedConnections connections
- * available, only superusers can make new connections" --- pre-existing
- * superuser connections don't count against the limit.
+ * superuser use, and ReservedConnections is the number of backends reserved
+ * for use by roles with privileges of the pg_use_reserved_connections
+ * 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_connections is
+ * (MaxConnections - SuperuserReservedConnections - ReservedConnections).
+ *
+ * If the number of remaining slots is less than or equal to
+ * SuperuserReservedConnections, only superusers can make new connections. If
+ * the number of remaining slots is greater than SuperuserReservedConnections
+ * but less than or equal to
+ * (SuperuserReservedConnections + ReservedConnections), only superusers and
+ * roles with privileges of pg_use_reserved_connections can make new
+ * connections. Note that pre-existing superuser and
+ * pg_use_reserved_connections connections don't count against the limits.
*/
int SuperuserReservedConnections;
+int ReservedConnections;
/* The socket(s) we're listening to. */
#define MAXLISTEN 64
@@ -908,11 +918,12 @@ PostmasterMain(int argc, char *argv[])
/*
* Check for invalid combinations of GUC settings.
*/
- if (SuperuserReservedConnections >= MaxConnections)
+ if (SuperuserReservedConnections + ReservedConnections >= 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,
- SuperuserReservedConnections, MaxConnections);
+ SuperuserReservedConnections, ReservedConnections,
+ 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 40f145e0ab..2f07ca7a0e 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,30 @@ 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.
+ *
+ * Note: At this point, the new backend has already claimed a proc struct,
+ * so we must check whether the number of free slots is strictly less than
+ * the reserved connection limits.
*/
if (!am_superuser && !am_walsender &&
- SuperuserReservedConnections > 0 &&
- !HaveNFreeProcs(SuperuserReservedConnections))
- ereport(FATAL,
- (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
- errmsg("remaining connection slots are reserved for superusers")));
+ (SuperuserReservedConnections + ReservedConnections) > 0 &&
+ !HaveNFreeProcs(SuperuserReservedConnections + ReservedConnections, &nfree))
+ {
+ if (nfree < SuperuserReservedConnections)
+ 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_connections")));
+ }
/* 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 0fa9fdd3c5..e1753a40fa 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
+ },
+ &ReservedConnections,
+ 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..d06074b86f 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -63,6 +63,7 @@
# (change requires restart)
#port = 5432 # (change requires restart)
#max_connections = 100 # (change requires restart)
+#reserved_connections = 0 # (change requires restart)
#superuser_reserved_connections = 3 # (change requires restart)
#unix_socket_directories = '/tmp' # comma-separated list of directories
# (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 0e4b8ded34..3b3889c58c 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 SuperuserReservedConnections;
+extern PGDLLIMPORT int ReservedConnections;
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-19 20:13 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 24+ messages in thread
From: Robert Haas @ 2023-01-19 20:13 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
On Thu, Jan 19, 2023 at 2:46 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> > Thanks. I'd move it to the inner indentation level so it's closer to
> > the test at issue.
>
> I meant for it to cover the call to HaveNFreeProcs() as well since the same
> idea applies. I left it the same for now, but if you still think it makes
> sense to move it, I'll do so.
Hmm, OK. If you want to leave it where it is, I won't argue further.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-20 13:34 tushar <tushar.ahuja@enterprisedb.com>
parent: tushar <tushar.ahuja@enterprisedb.com>
1 sibling, 1 reply; 24+ messages in thread
From: tushar @ 2023-01-20 13:34 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On 1/19/23 6:28 PM, tushar wrote:
>>
> There is one typo , for the doc changes, it is mentioned
> "pg_use_reserved_backends" but i think it supposed to be
> "pg_use_reserved_connections"
> under Table 22.1. Predefined Roles.
Thanks, this is fixed now with the latest patches.
--
regards,tushar
EnterpriseDB https://www.enterprisedb.com/
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-20 18:10 Nathan Bossart <nathandbossart@gmail.com>
parent: tushar <tushar.ahuja@enterprisedb.com>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-01-20 18:10 UTC (permalink / raw)
To: tushar <tushar.ahuja@enterprisedb.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
On Fri, Jan 20, 2023 at 07:04:58PM +0530, tushar wrote:
> On 1/19/23 6:28 PM, tushar wrote:
>> There is one typo , for the doc changes, it is mentioned
>> "pg_use_reserved_backends" but i think it supposed to be
>> "pg_use_reserved_connections"
>> under Table 22.1. Predefined Roles.
>
> Thanks, this is fixed now with the latest patches.
Thank you for reviewing.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-20 20:42 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Robert Haas @ 2023-01-20 20:42 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: tushar <tushar.ahuja@enterprisedb.com>; pgsql-hackers
On Fri, Jan 20, 2023 at 1:10 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> > Thanks, this is fixed now with the latest patches.
>
> Thank you for reviewing.
Thanks to you both. I have committed these patches.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-20 21:02 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-01-20 21:02 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: tushar <tushar.ahuja@enterprisedb.com>; pgsql-hackers
On Fri, Jan 20, 2023 at 03:42:03PM -0500, Robert Haas wrote:
> Thanks to you both. I have committed these patches.
Thanks! Does this need a catversion bump?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet
@ 2023-01-20 21:37 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 24+ messages in thread
From: Robert Haas @ 2023-01-20 21:37 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: tushar <tushar.ahuja@enterprisedb.com>; pgsql-hackers
On Fri, Jan 20, 2023 at 4:02 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> On Fri, Jan 20, 2023 at 03:42:03PM -0500, Robert Haas wrote:
> > Thanks to you both. I have committed these patches.
>
> Thanks! Does this need a catversion bump?
I was surprised by this question because I thought I'd included one.
But it turns out I didn't include that in the commit and it's still in
my working tree. *facepalm*
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
end of thread, other threads:[~2023-01-20 21:37 UTC | newest]
Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-01-16 19:29 almost-super-user problems that we haven't fixed yet Robert Haas <robertmhaas@gmail.com>
2023-01-16 22:37 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-17 02:06 ` Robert Haas <robertmhaas@gmail.com>
2023-01-17 18:42 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-17 19:59 ` Robert Haas <robertmhaas@gmail.com>
2023-01-18 00:15 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-18 16:28 ` Robert Haas <robertmhaas@gmail.com>
2023-01-18 19:00 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-18 19:51 ` Robert Haas <robertmhaas@gmail.com>
2023-01-18 21:14 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-19 12:58 ` tushar <tushar.ahuja@enterprisedb.com>
2023-01-19 13:20 ` tushar <tushar.ahuja@enterprisedb.com>
2023-01-19 14:21 ` tushar <tushar.ahuja@enterprisedb.com>
2023-01-19 15:31 ` Robert Haas <robertmhaas@gmail.com>
2023-01-20 13:34 ` tushar <tushar.ahuja@enterprisedb.com>
2023-01-20 18:10 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-20 20:42 ` Robert Haas <robertmhaas@gmail.com>
2023-01-20 21:02 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-20 21:37 ` Robert Haas <robertmhaas@gmail.com>
2023-01-19 16:40 ` Robert Haas <robertmhaas@gmail.com>
2023-01-19 17:54 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-19 19:17 ` Robert Haas <robertmhaas@gmail.com>
2023-01-19 19:46 ` Nathan Bossart <nathandbossart@gmail.com>
2023-01-19 20:13 ` Robert Haas <robertmhaas@gmail.com>
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