TODO Done. Superuser backend slot reservations
TODO item:
Administration -
Reserve last few process slots for super-user if max_connections
reached
Notes:
Added GUC superuser_reserved_connections such that non-superuser connections
are only acceptable in the first
(max_connections - superuser_reserved_connections) backend slots.
Superuser connections within these first n slots count towards this
non-superuser connection limit. Therefore there can be at most this number
of non-superuser connections but may be less.
In addition, this limit is only checked on initialisation of a backend
process. So reserved slots can be taken by connections that subsequently
lose superuser priviledges thus evading the lower limit on backends.
Passed regression tests, not that it was likely not to.
Behaved as expected in a manual test.
--
Nigel J. Andrews
Director
---
Logictree Systems Limited
Computer Consultants
Attachments:
sureserve.patchtext/plain; charset=US-ASCII; name=sureserve.patchDownload
? config.log
? GNUmakefile
? config.status
? src/Makefile.global
? src/include/pg_config.h
? src/include/stamp-h
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
retrieving revision 1.285
diff -c -r1.285 postmaster.c
*** src/backend/postmaster/postmaster.c 2002/08/18 03:03:25 1.285
--- src/backend/postmaster/postmaster.c 2002/08/25 22:27:47
***************
*** 151,156 ****
--- 151,168 ----
*/
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.
+ */
+ int ReservedBackends = 2;
+
static char *progname = (char *) NULL;
***************
*** 566,571 ****
--- 578,591 ----
SetDataDir(potential_DataDir);
ProcessConfigFile(PGC_POSTMASTER);
+
+ /*
+ * Force ReservedBackends is less than MaxBackends if need be.
+ * A cluster only allowing superuser connections seems silly whereas
+ * a cluster reserving none for superusers doesn't.
+ */
+ if (ReservedBackends >= MaxBackends)
+ ReservedBackends = MaxBackends - 1;
/*
* Now that we are done processing the postmaster arguments, reset
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/init/postinit.c,v
retrieving revision 1.109
diff -c -r1.109 postinit.c
*** src/backend/utils/init/postinit.c 2002/07/20 05:16:59 1.109
--- src/backend/utils/init/postinit.c 2002/08/25 22:27:48
***************
*** 402,407 ****
--- 402,417 ----
/* close the transaction we started above */
if (!bootstrap)
CommitTransactionCommand();
+
+ /*
+ * 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 (!superuser() && MyBackendId > MaxBackends - ReservedBackends)
+ elog(ERROR, "Normal user limit exceeded");
}
/*
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.83
diff -c -r1.83 guc.c
*** src/backend/utils/misc/guc.c 2002/08/18 03:03:25 1.83
--- src/backend/utils/misc/guc.c 2002/08/25 22:27:51
***************
*** 537,547 ****
/*
* Note: There is some postprocessing done in PostmasterMain() to make
* sure the buffers are at least twice the number of backends, so the
! * constraints here are partially unused.
*/
{
{ "max_connections", PGC_POSTMASTER }, &MaxBackends,
DEF_MAXBACKENDS, 1, INT_MAX, NULL, NULL
},
{
--- 537,553 ----
/*
* Note: There is some postprocessing done in PostmasterMain() to make
* sure the buffers are at least twice the number of backends, so the
! * constraints here are partially unused. Also the super user reserved
! * number is forced to less than the max backends number there.
*/
{
{ "max_connections", PGC_POSTMASTER }, &MaxBackends,
DEF_MAXBACKENDS, 1, INT_MAX, NULL, NULL
+ },
+
+ {
+ { "superuser_reserved_connections", PGC_POSTMASTER }, &ReservedBackends,
+ 2, 0, INT_MAX, NULL, NULL
},
{
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.45
diff -c -r1.45 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample 2002/08/18 03:03:26 1.45
--- src/backend/utils/misc/postgresql.conf.sample 2002/08/25 22:27:51
***************
*** 31,36 ****
--- 31,37 ----
#ssl = false
#max_connections = 32
+ #superuser_reserved_connections = 2
#port = 5432
#hostname_lookup = false
Index: src/include/miscadmin.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/include/miscadmin.h,v
retrieving revision 1.106
diff -c -r1.106 miscadmin.h
*** src/include/miscadmin.h 2002/06/20 20:29:42 1.106
--- src/include/miscadmin.h 2002/08/25 22:27:52
***************
*** 179,184 ****
--- 179,185 ----
extern bool EnableSSL;
extern bool SilentMode;
extern int MaxBackends;
+ extern int ReservedBackends;
extern int NBuffers;
extern int PostPortNumber;
extern int Unix_socket_permissions;
[only replying to -patches, this doesn't belong on -hackers AFAICS ]
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
Added GUC superuser_reserved_connections such that non-superuser connections
are only acceptable in the first
(max_connections - superuser_reserved_connections) backend slots.
I'd prefer that we not change the meaning of max_connections. I had in
mind the following:
- max_connections denotes the # of "regular" connections to
the database (i.e. admin & non-admin)
- max_admin_connections denotes an additional # of backend
slots that are reserved for connections from superusers. Not
sure if this should default to zero or not
- Therefore, the # of backend slots created is
(max_connections + max_admin_connections)
In addition, this limit is only checked on initialisation of a backend
process. So reserved slots can be taken by connections that subsequently
lose superuser priviledges thus evading the lower limit on backends.
How can that happen?
+ /*
+ * Force ReservedBackends is less than MaxBackends if need be.
+ * A cluster only allowing superuser connections seems silly whereas
+ * a cluster reserving none for superusers doesn't.
+ */
+ if (ReservedBackends >= MaxBackends)
+ ReservedBackends = MaxBackends - 1;IMHO, we should elog(FATAL) here, or at least emit a warning.
Cheers,
Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
Neil Conway wrote:
[only replying to -patches, this doesn't belong on -hackers AFAICS ]
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
Added GUC superuser_reserved_connections such that non-superuser connections
are only acceptable in the first
(max_connections - superuser_reserved_connections) backend slots.I'd prefer that we not change the meaning of max_connections. I had in
mind the following:- max_connections denotes the # of "regular" connections to
the database (i.e. admin & non-admin)
Well, if you do that, then max_connections is really not max
connections, it is maximum connections minus admin_connections. That
seems confusing.
I think I prefer just reserving 1-2 of the last slots. The kernel's
proc code is the same; if you specify a proc table of 150, it is 150,
with 149 and 150 reserved for root.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073On Sun, 25 Aug 2002, Bruce Momjian wrote:
Neil Conway wrote:
[only replying to -patches, this doesn't belong on -hackers AFAICS ]
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
Added GUC superuser_reserved_connections such that non-superuser connections
are only acceptable in the first
(max_connections - superuser_reserved_connections) backend slots.I'd prefer that we not change the meaning of max_connections. I had in
mind the following:- max_connections denotes the # of "regular" connections to
the database (i.e. admin & non-admin)Well, if you do that, then max_connections is really not max
connections, it is maximum connections minus admin_connections. That
seems confusing.
That was my thinking. Plus, MaxBackends gets used in quite a few places, to
avoid having to go to extremes and look at and possibly hit each of those
places to ensure things are correct seems silly. An alternative would be to
change the variable max_connections is tied to and make MaxBackends the
sum of two GUCs. I couldn't see the point in doing either of those when
max_connections should define the maximum number of connections possible
and not something less.
Hmmm...rereading that it strikes me that we're all reading from the same
page of the same book. All three of us are saying max_connections gives
the maximum number of connections possible from admin and non-admin users
together.
I think I prefer just reserving 1-2 of the last slots. The kernel's
proc code is the same; if you specify a proc table of 150, it is 150,
with 149 and 150 reserved for root.
Yes, only the exact number reserved at the top of the slot array is
configurable via the GUC variable.
--
Nigel J. Andrews
On 25 Aug 2002, Neil Conway wrote:
[only replying to -patches, this doesn't belong on -hackers AFAICS ]
Is -patches for discussions? I thought it was only for the patches
themselves, I'd better go subscribe...
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
In addition, this limit is only checked on initialisation of a backend
process. So reserved slots can be taken by connections that subsequently
lose superuser priviledges thus evading the lower limit on backends.How can that happen?
Well, the test is located somewhere that is only called once, when the backend
process is forked. At least that's what I think but as I say I'm not 100%
certain, mostly becuase I haven't checked only looked to see what the
routine is doing and it looks like a one shot routine to me. Therefore,
assuming doing a SET SESSION AUTH... doesn't drop and then reconnect to the
server, a change from a superuser to a normaluser is not going to result in a
dropped connection. Nor should it do I believe.
+ /* + * Force ReservedBackends is less than MaxBackends if need be. + * A cluster only allowing superuser connections seems silly whereas + * a cluster reserving none for superusers doesn't. + */ + if (ReservedBackends >= MaxBackends) + ReservedBackends = MaxBackends - 1;IMHO, we should elog(FATAL) here, or at least emit a warning.
The warning sounds reasonable to me. I'll add one and resubmit in a day or two
after I've seen what else gets said.
--
Nigel J. Andrews
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
Reserve last few process slots for super-user if max_connections
reached
Another minor coding suggestion:
+ /*
+ * 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 (!superuser() && MyBackendId > MaxBackends - ReservedBackends)
+ elog(ERROR, "Normal user limit exceeded");This would be more efficient if you placed the superuser() test after
the BackendId test, as it is both more expensive to evaluate and more
likely to be true.
Cheers,
Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
Neil Conway <neilc@samurai.com> writes:
- Therefore, the # of backend slots created is
(max_connections + max_admin_connections)
I tend to agree with Bruce on this: max_connections means
max_connections. Therefore, the number of backend slots is
max_connections, of which max_connections - max_admin_connections
are available to non-superusers.
(There is provision in the existing code for one extra child process
for checkpoints, but it's not a "real" backend and so it's reasonable
not to count it against max_connections.)
regards, tom lane
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
+ if (!superuser() && MyBackendId > MaxBackends - ReservedBackends) + elog(ERROR, "Normal user limit exceeded");
This coding is wrong on its face: the slot number you happen to find has
no relationship to the number of slots remaining free, except as an
existence proof that the number of slots free was > 0 before you took
one.
regards, tom lane
On Mon, 26 Aug 2002, Tom Lane wrote:
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
+ if (!superuser() && MyBackendId > MaxBackends - ReservedBackends) + elog(ERROR, "Normal user limit exceeded");This coding is wrong on its face: the slot number you happen to find has
no relationship to the number of slots remaining free, except as an
existence proof that the number of slots free was > 0 before you took
one.
Yes.
I was taking the line that the last slots in the array are reserved. Those are
not going to be taken by non su connections. Therefore, if MyBackendId is
under the lower limit it doesn't matter if it's the only slot free since the
'safety' measure has already been used in restricting access to the last free
slots and it just so happens that those sessions are still active.
I take Neil's point about the order of the tests. That's my stupidity when
rearranging stuff after noticing in tests that the user information wasn't
available where I was [also stupidly] expecting it to be first time around.
--
Nigel J. Andrews
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
I was taking the line that the last slots in the array are
reserved. Those are not going to be taken by non su connections.
But that doesn't do the job, does it? My view of the feature is that
when there are at least MaxBackends - ReservedBackends slots in use (by
either su or non-su connections) then no new non-su jobs should be let
in. For example, if the system is full (with a mix of su and non-su
jobs) and one non-su job quits, don't we want to hold that slot for a
possible su connection?
Your approach does have the advantage of being very cheap to test
(I think my semantics would require counting the active backends),
but I'm not sure that it really does what we want.
regards, tom lane
Tom Lane wrote:
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
I was taking the line that the last slots in the array are
reserved. Those are not going to be taken by non su connections.But that doesn't do the job, does it? My view of the feature is that
when there are at least MaxBackends - ReservedBackends slots in use (by
either su or non-su connections) then no new non-su jobs should be let
in. For example, if the system is full (with a mix of su and non-su
jobs) and one non-su job quits, don't we want to hold that slot for a
possible su connection?Your approach does have the advantage of being very cheap to test
(I think my semantics would require counting the active backends),
but I'm not sure that it really does what we want.
Tom is right. If the last two slots are held by two long-running
super-user backends, and the slots fill, there will be no reserved
slots. The trick is that when the maximum number of backends is almost
exceeded, only let the supuer-user in.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073On Mon, 26 Aug 2002, Bruce Momjian wrote:
Tom Lane wrote:
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
I was taking the line that the last slots in the array are
reserved. Those are not going to be taken by non su connections.But that doesn't do the job, does it? My view of the feature is that
when there are at least MaxBackends - ReservedBackends slots in use (by
either su or non-su connections) then no new non-su jobs should be let
in. For example, if the system is full (with a mix of su and non-su
jobs) and one non-su job quits, don't we want to hold that slot for a
possible su connection?Your approach does have the advantage of being very cheap to test
(I think my semantics would require counting the active backends),
but I'm not sure that it really does what we want.Tom is right. If the last two slots are held by two long-running
super-user backends, and the slots fill, there will be no reserved
slots. The trick is that when the maximum number of backends is almost
exceeded, only let the supuer-user in.
Okay, it's not how I was thinking as you know but I've got nothing against it
other than the backend slot scan time. I don't think that would be a
significant drain of cpu time so I'll implement that scheme and resubmit.
Got some other stuff to do first so it won't be done immediately but will in
the next day or so; in time for beta assuming it doesn't fall foul of any patch
review interval required.
--
Nigel J. Andrews