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
>From 94ee89548fd1080447b784993a1418480f407b49 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathandboss...@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
>From ba7e566a097fd5ffbc20f67a9caccea31ef19212 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathandboss...@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