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
Sun, 25 Aug 2002 23:56:31 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
TODO Done. Superuser backend slot reservations
[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


25 Aug 2002 19:26:06 -0400
Neil Conway <neilc@samurai.com>
Re: [HACKERS] TODO Done. Superuser backend slot reservations
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 19073

Sun, 25 Aug 2002 19:31:13 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] TODO Done. Superuser backend slot reservations
On 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


Mon, 26 Aug 2002 00:59:01 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
Re: [HACKERS] TODO Done. Superuser backend slot reservations
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



Mon, 26 Aug 2002 01:15:10 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
Re: [HACKERS] TODO Done. Superuser backend slot reservations
"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


25 Aug 2002 21:41:34 -0400
Neil Conway <neilc@samurai.com>
Re: [HACKERS] TODO Done. Superuser backend slot reservations
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

Sun, 25 Aug 2002 23:56:04 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] TODO Done. Superuser backend slot reservations 
"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

Mon, 26 Aug 2002 00:04:24 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] TODO Done. Superuser backend slot reservations 

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


Mon, 26 Aug 2002 11:36:10 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
Re: [HACKERS] TODO Done. Superuser backend slot reservations
"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

Mon, 26 Aug 2002 10:00:48 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] TODO Done. Superuser backend slot reservations 
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 19073

Mon, 26 Aug 2002 12:16:32 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] TODO Done. Superuser backend slot reservations
On 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


Mon, 26 Aug 2002 23:58:00 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
Re: [HACKERS] TODO Done. Superuser backend slot reservations