limiting connections per user/database

Petr Jel�nek said:

Something about patch:
I added two new guc variables name max_db_connections and
max_user_connections which can be set by superuser which means it can
be in main config file or in user/database config.

Is this what is intended by the TODO item? I thought that it was intended to
allow max connections to be specified on a per-db or per-user basis, not
just for global limits on per-user or per-db connections.

cheers

andrew

=?ISO-8859-2?Q?Petr_Jel=EDnek?= <pjmodos@parba.cz> writes:

patch I attached allows to set connectin limits per user or per database
(it's on TODO list).

I don't think this is going to work. The first problem with it is that
it changes pgstats from an optional into a required part of the system.
The second is that since the pgstats output lags well behind the actual
state of the system, it'll be at best a very approximate limit on the
number of connections. Perhaps for some people that's good enough, but
I kinda doubt it's what the people asking for the feature have in mind.

regards, tom lane

Ok didn't know that stats are well behind actual state - I used pgstat
because it already stores number of conenctions per database so I
thought doing same thing again is needless.
So you think I should store those data somewhere myself. What you think
is best place, some extra hash or in system catalog or ... ?

BTW you should still apply that pgstat.c.diff because it fixes bug in
current CVS

--
Regards
Petr Jelinek (PJMODOS)
www.parba.cz

Andrew Dunstan wrote:

Is this what is intended by the TODO item? I thought that it was intended to
allow max connections to be specified on a per-db or per-user basis, not
just for global limits on per-user or per-db connections.

They are - ALTER DATABASE dbname SET max_db_conenctions = '10';
Like I said they're checked after user specific and database specific
config is loaded.
But after what Tom said I think I will have to rewrite it all so it
doesn't really matter.

--
Regards
Petr Jelinek (PJMODOS)

=?windows-1250?Q?Petr_Jel=EDnek?= <pjmodos@parba.cz> writes:

BTW you should still apply that pgstat.c.diff because it fixes bug in
current CVS

What bug exactly?

regards, tom lane

Tom Lane wrote:

What bug exactly?

Database stats aren't initialized so everything in pg_stat_database is
always zero.

--
Regards
Petr Jelinek (PJMODOS)

=?windows-1250?Q?Petr_Jel=EDnek?= <pjmodos@parba.cz> writes:

Tom Lane wrote:

What bug exactly?

Database stats aren't initialized so everything in pg_stat_database is
always zero.

[ shrug... ] Don't see that here; sure it isn't something broken in
your local modified version?

regards, tom lane

Tom Lane napsal(a):

[ shrug... ] Don't see that here; sure it isn't something broken in
your local modified version?

Well I tryed it with unmodified cvs copy and I have numbackends is 0
when 40 clients are connected (using default config)

I am bit confused now because I am no really sure if it's intended to be
this way or not - 8.0 behaviour was to report numbackends when stats
were on, now it reports numbackends when stats_row_level is true.
I should prolly talk with neilc to see what he intended to do when he
commited code which changed this (but I believe it's side effect not
intended change)

--
Regards
Petr Jelinek (PJMODOS)

One more thing, do you have more concerns about that max connection out
of the fact it uses pgstat ?
For example are guc variables ok for this or should it be new variable
in pg_shadow & pg_database etc - I would like to rewrite it just once or
twice not ten times.

--
Regards
Petr Jelinek (PJMODOS)

Hi Petr,

One more thing, do you have more concerns about that max connection
out
of the fact it uses pgstat ?
For example are guc variables ok for this or should it be new
variable
in pg_shadow & pg_database etc - I would like to rewrite it just once
or
twice not ten times.

IIRC, people want the last one. We have more control if we can set it
per user or per database. Talking about pgstat, I think you can rely on
it 'cause it can be disabled. Could you describe the design you
intended to implement?

Euler Taveira de Oliveira
euler[at]yahoo_com_br

__________________________________________________
Converse com seus amigos em tempo real com o Yahoo! Messenger
http://br.download.yahoo.com/messenger/

Euler Taveira de Oliveira wrote:

IIRC, people want the last one. We have more control if we can set it
per user or per database.

Like I said, guc variables _can_ be set per user or per database if you
use right context.

Talking about pgstat, I think you can rely on
it 'cause it can be disabled. Could you describe the design you
intended to implement?

I "can rely on it 'cause it can be disabled" - did you mean that I can't
rely on it ?
Well I am still not sure how I will implement it when I can't use
pgstat. I think that I will have to use similar aproach which makes me sad.

--
Regards
Petr Jelinek (PJMODOS)

On Sun, Jun 26, 2005 at 07:46:32PM +0200, Petr Jel�nek wrote:

Euler Taveira de Oliveira wrote:

IIRC, people want the last one. We have more control if we can set it
per user or per database.

Like I said, guc variables _can_ be set per user or per database if you
use right context.

I don't think this approach is very user-friendly. I'd vote for the
catalog approach, I think.

Talking about pgstat, I think you can rely on
it 'cause it can be disabled. Could you describe the design you
intended to implement?

I "can rely on it 'cause it can be disabled" - did you mean that I can't
rely on it ?
Well I am still not sure how I will implement it when I can't use
pgstat. I think that I will have to use similar aproach which makes me sad.

Maybe you could make some checks against the shared array of PGPROCs
(procarray.c), for the per-database limit at least. Not sure about
per-user limit.

--
Alvaro Herrera (<alvherre[a]surnet.cl>)
"[PostgreSQL] is a great group; in my opinion it is THE best open source
development communities in existence anywhere." (Lamar Owen)

Alvaro Herrera wrote:

I don't think this approach is very user-friendly. I'd vote for the
catalog approach, I think.

Ok I am fine with both but catalog changes would mean more hacking of
ALTER DATABASE and ALTER USER.

Maybe you could make some checks against the shared array of PGPROCs
(procarray.c), for the per-database limit at least. Not sure about
per-user limit.

Thats good idea (I could maybe add userid to PGPROC struct too) but I
think there could be problem with two phase commits because they add new
entry to that array of PGPROCs too and I don't kow if we want to include
them to that limit.

--
Regards
Petr Jelinek (PJMODOS)

On Sun, 26 Jun 2005, Petr Jel�nek wrote:

Alvaro Herrera wrote:

Maybe you could make some checks against the shared array of PGPROCs
(procarray.c), for the per-database limit at least. Not sure about
per-user limit.

Thats good idea (I could maybe add userid to PGPROC struct too) but I think
there could be problem with two phase commits because they add new entry to
that array of PGPROCs too and I don't kow if we want to include them to that
limit.

You can ignore PGPROCs that belong to prepared transactions. They have 0
in the pid field, see TransactionIdIsActive and
CountActiveBackends functions in procarray.c for an example.

- Heikki

On Sun, Jun 26, 2005 at 08:52:55PM +0200, Petr Jel�nek wrote:

Alvaro Herrera wrote:

Maybe you could make some checks against the shared array of PGPROCs
(procarray.c), for the per-database limit at least. Not sure about
per-user limit.

Thats good idea (I could maybe add userid to PGPROC struct too) but I
think there could be problem with two phase commits because they add new
entry to that array of PGPROCs too and I don't kow if we want to include
them to that limit.

Prepared transactions can be filtered out by checking the pid struct
member. I'm not sure if anybody would object to adding the
authenticated user Id to ProcArray, but I don't see why not.

--
Alvaro Herrera (<alvherre[a]surnet.cl>)
"In Europe they call me Niklaus Wirth; in the US they call me Nickel's worth.
That's because in Europe they call me by name, and in the US by value!"

Alvaro Herrera wrote:

Prepared transactions can be filtered out by checking the pid struct
member. I'm not sure if anybody would object to adding the
authenticated user Id to ProcArray, but I don't see why not.

Very well, it seems to work this way (although the code for storing
userid in PGPROC isn't as clean as I hoped).

The problem now is that I am storing those limits in system catalog but
there is no ALTER DATABASE implementation which I could use to change
this - there are only RENAME, OWNER and SET implementations and those
are not usable for "normal" properties (SET is for guc variables only
which was the actual reason for me to use guc variables first time).
That said I think that I will have to implement some ALTER DATABASE
command for those purposes and I am not sure if I can handle it because
I am not familiar with bison (I have even problems to add new ALTER USER
property to existing implementation).

--
Regards
Petr Jelinek (PJMODOS)

Petr Jel�nek wrote:

I am bit confused now because I am no really sure if it's intended to be
this way or not - 8.0 behaviour was to report numbackends when stats
were on, now it reports numbackends when stats_row_level is true.

Yeah, this is a bug. Attached is a fix. I'll apply this to HEAD later
today barring any objections.

-Neil

Neil Conway <neilc@samurai.com> writes:

Yeah, this is a bug. Attached is a fix. I'll apply this to HEAD later
today barring any objections.

I looked at this but did not actually see the code path that requires
forcing creation of the per-DB entry right at this spot. The HASH_FIND
calls for this hashtable seem to all happen on the backend side not the
collector side. Can you explain why we need this?

regards, tom lane

Tom Lane wrote:

I looked at this but did not actually see the code path that requires
forcing creation of the per-DB entry right at this spot. The HASH_FIND
calls for this hashtable seem to all happen on the backend side not the
collector side. Can you explain why we need this?

Yeah, I missed this when making the original change (this code is rather
opaque :-\). The problem is that if we don't initialize the dbentry for
the database we connect to, it won't get written out to the statsfile in
pgstat_write_statsfile(). So the database won't be counted as having any
backends connected to it in pgstat_read_statsfile() (see line 2558 of
pgstat.c in HEAD).

BTW, the comment at line 2210 of pgstat.c is misleading: the n_backends
in the entries of the dbentry hash table are explicitly ignored when
reading in the stats file -- the value is instead derived from the
number of beentries that are seen.

-Neil