From 8362be35e82574801113a7fe4dfdc3037010fc04 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 21 Nov 2002 06:36:08 +0000 Subject: [PATCH] Code review for superuser_reserved_connections patch. Don't try to do database access outside a transaction; revert bogus performance improvement in SIBackendInit(); improve comments; add documentation (this part courtesy Neil Conway). --- doc/src/sgml/runtime.sgml | 55 ++++++++++++++++++++--------- src/backend/postmaster/postmaster.c | 36 ++++++++++--------- src/backend/storage/ipc/sinval.c | 13 ++++--- src/backend/storage/ipc/sinvaladt.c | 24 +++++++------ src/backend/utils/init/postinit.c | 25 ++++++------- 5 files changed, 90 insertions(+), 63 deletions(-) diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index 70b29118a2..7ecd951a3b 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -1,5 +1,5 @@ @@ -1902,6 +1902,28 @@ dynamic_library_path = '/usr/local/lib/postgresql:/home/my_project/lib:$libdir' + + SUPERUSER_RESERVED_CONNECTIONS + (integer) + + + Determines the number of connection slots that + are reserved for connections by PostgreSQL + superusers. At most max_connections connections can + ever be active simultaneously. Whenever the number of active + concurrent connections is at least max_connections minus + superuser_reserved_connections, new connections + will be accepted only from superuser accounts. + + + + The default value is 2. The value must be less than the value of + max_connections. This parameter can only be + set at server start. + + + + TCPIP_SOCKET (boolean) @@ -2952,24 +2974,25 @@ $ kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid` - With SSL support compiled in, the PostgreSQL server - can be started with SSL support by setting the parameter - ssl to on in - postgresql.conf. When starting in SSL mode, - the server will look for the files server.key and - server.crt in the data directory. These files should - contain the server private key and certificate respectively. These - files must be set up correctly before an SSL-enabled server can - start. If the private key is protected with a passphrase, the - server will prompt for the passphrase and will not start until it - has been entered. + With SSL support compiled in, the + PostgreSQL server can be started with + SSL support by setting the parameter + ssl to on in postgresql.conf. When + starting in SSL mode, the server will look for the + files server.key and server.crt in the + data directory. These files should contain the server private key + and certificate respectively. These files must be set up correctly + before an SSL-enabled server can start. If the private key is + protected with a passphrase, the server will prompt for the + passphrase and will not start until it has been entered. - The server will listen for both standard and SSL connections on the - same TCP/IP port, and will negotiate with any connecting client on - whether to use SSL. See about - how to force the server to only use of SSL for certain connections. + The server will listen for both standard and SSL + connections on the same TCP/IP port, and will negotiate with any + connecting client on whether to use SSL. See about how to force the server to + require use of SSL for certain connections. diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 187f0191c9..8f34a3fd2d 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -37,7 +37,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v 1.298 2002/11/18 00:40:46 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v 1.299 2002/11/21 06:36:08 tgl Exp $ * * NOTES * @@ -154,12 +154,11 @@ int MaxBackends = DEF_MAXBACKENDS; /* * ReservedBackends is the number of backends reserved for superuser use. * This number is taken out of the pool size given by MaxBackends so - * number of backend slots available to none super users is - * (MaxBackends - ReservedBackends). Note, existing super user - * connections are not taken into account once this lower limit has - * been reached, i.e. superuser connections made before the lower limit - * is reached always count towards that limit and are not taken from - * ReservedBackends. + * number of backend slots available to non-superusers is + * (MaxBackends - 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. */ int ReservedBackends = 2; @@ -568,7 +567,15 @@ PostmasterMain(int argc, char *argv[]) } /* - * Check for invalid combinations of switches + * Now we can set the data directory, and then read postgresql.conf. + */ + checkDataDir(potential_DataDir); /* issues error messages */ + SetDataDir(potential_DataDir); + + ProcessConfigFile(PGC_POSTMASTER); + + /* + * Check for invalid combinations of GUC settings. */ if (NBuffers < 2 * MaxBackends || NBuffers < 16) { @@ -581,16 +588,11 @@ PostmasterMain(int argc, char *argv[]) ExitPostmaster(1); } - checkDataDir(potential_DataDir); /* issues error messages */ - SetDataDir(potential_DataDir); - - ProcessConfigFile(PGC_POSTMASTER); - - /* - * Force an exit if ReservedBackends is not less than MaxBackends. - */ if (ReservedBackends >= MaxBackends) - elog(FATAL, "superuser_reserved_connections must be less than max_connections."); + { + postmaster_error("superuser_reserved_connections must be less than max_connections."); + ExitPostmaster(1); + } /* * Now that we are done processing the postmaster arguments, reset diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c index 87f7a29245..239c3bc8d3 100644 --- a/src/backend/storage/ipc/sinval.c +++ b/src/backend/storage/ipc/sinval.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/storage/ipc/sinval.c,v 1.52 2002/09/04 20:31:25 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/storage/ipc/sinval.c,v 1.53 2002/11/21 06:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -542,12 +542,11 @@ BackendIdGetProc(BackendId procId) /* * CountEmptyBackendSlots - count empty slots in backend process table * - * Doesn't count since the procState array could be large and we've already - * allowed for that by running a freeBackends counter in the SI segment. - * Unlike CountActiveBackends() we do not need to interrogate the - * backends to determine the free slot count. - * Goes for a lock despite being a trival look up in case other backends - * are busy starting or exiting since there is scope for confusion. + * We don't actually need to count, since sinvaladt.c maintains a + * freeBackends counter in the SI segment. + * + * Acquiring the lock here is almost certainly overkill, but just in + * case fetching an int is not atomic on your machine ... */ int CountEmptyBackendSlots(void) diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index b4ab1689f9..6850fa8cf5 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/src/backend/storage/ipc/sinvaladt.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/storage/ipc/sinvaladt.c,v 1.48 2002/08/29 21:02:12 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/storage/ipc/sinvaladt.c,v 1.49 2002/11/21 06:36:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -92,13 +92,6 @@ SIBackendInit(SISeg *segP) int index; ProcState *stateP = NULL; - if (segP->freeBackends == 0) - { - /* out of procState slots */ - MyBackendId = InvalidBackendId; - return 0; - } - /* Look for a free entry in the procState array */ for (index = 0; index < segP->lastBackend; index++) { @@ -111,9 +104,18 @@ SIBackendInit(SISeg *segP) if (stateP == NULL) { - stateP = &segP->procState[segP->lastBackend]; - Assert(stateP->nextMsgNum < 0); - segP->lastBackend++; + if (segP->lastBackend < segP->maxBackends) + { + stateP = &segP->procState[segP->lastBackend]; + Assert(stateP->nextMsgNum < 0); + segP->lastBackend++; + } + else + { + /* out of procState slots */ + MyBackendId = InvalidBackendId; + return 0; + } } MyBackendId = (stateP - &segP->procState[0]) + 1; diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 9ac71c6a81..3ad2c50a84 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v 1.117 2002/10/03 19:19:09 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v 1.118 2002/11/21 06:36:08 tgl Exp $ * * *------------------------------------------------------------------------- @@ -377,6 +377,18 @@ InitPostgres(const char *dbname, const char *username) */ RelationCacheInitializePhase3(); + /* + * Check a normal user hasn't connected to a superuser reserved slot. + * We can't do this till after we've read the user information, and + * we must do it inside a transaction since checking superuserness + * may require database access. The superuser check is probably the + * most expensive part; don't do it until necessary. + */ + if (ReservedBackends > 0 && + CountEmptyBackendSlots() < ReservedBackends && + !superuser()) + elog(FATAL, "Non-superuser connection limit exceeded"); + /* * Initialize various default states that can't be set up until we've * selected the active user and done ReverifyMyDatabase. @@ -397,17 +409,6 @@ InitPostgres(const char *dbname, const char *username) /* close the transaction we started above */ if (!bootstrap) CommitTransactionCommand(true); - - /* - * Check a normal user hasn't connected to a superuser reserved slot. - * Do this here since we need the user information and that only - * happens after we've started bringing the shared memory online. So - * we wait until we've registered exit handlers and potentially shut - * an open transaction down for an as safety conscious rejection as - * possible. - */ - if (CountEmptyBackendSlots() < ReservedBackends && !superuser()) - elog(ERROR, "Non-superuser connection limit exceeded"); } /*