Re: [PATCHES] per user/database connections limit again
Am Montag, 1. August 2005 16:08 schrieb Bruce Momjian: Would this not work in the context of the general user-specific ALTER USER ... SET something = something? No because it isn't a GUC variable, it is per-user/db value. GUC supports per-user/per-db values. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] per user/database connections limit again
Peter Eisentraut [EMAIL PROTECTED] writes: Am Montag, 1. August 2005 16:08 schrieb Bruce Momjian: Would this not work in the context of the general user-specific ALTER USER ... SET something = something? No because it isn't a GUC variable, it is per-user/db value. GUC supports per-user/per-db values. But not in the style that we'd want this to work. You couldn't just invent a single connection_limit variable, because a per-user setting would override a per-database setting, which is not the desired behavior. You'd have to invent two separate GUC variables, and there would be nothing except convention enforcing that they be set through ALTER USER and ALTER DATABASE rather than at other random places. We could do it that way, but it strikes me as messy and confusing, and I don't see any actual benefit other than saving a few lines of (already written) code. In what way would a GUC-based implementation be more useful than what's there? regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] per user/database connections limit again
Peter Eisentraut wrote: GUC supports per-user/per-db values. We already had discussion here about GUC for this and we agreed that catalog change is better than new GUC variable in this case. -- Regards Petr Jelinek (PJMODOS) ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] per user/database connections limit again
Am Montag, 25. Juli 2005 18:31 schrieb Tom Lane: Bruce Momjian pgman@candle.pha.pa.us writes: The new syntax for this command is CREATE/ALTER DATABASE/USER: | MAX CONNECTIONS Iconst This adds 'max' as a keyword, though at a fairly unreserved level, I think. Should we use the syntax LIMIT CONNECTIONS so we don't have to add MAX as a keyword at all? I didn't like that either. I was thinking of just CONNECTIONS. LIMIT CONNECTIONS sort of works grammatically, I guess. Would this not work in the context of the general user-specific ALTER USER ... SET something = something? -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] per user/database connections limit again
Peter Eisentraut wrote: Am Montag, 25. Juli 2005 18:31 schrieb Tom Lane: Bruce Momjian pgman@candle.pha.pa.us writes: The new syntax for this command is CREATE/ALTER DATABASE/USER: | MAX CONNECTIONS Iconst This adds 'max' as a keyword, though at a fairly unreserved level, I think. Should we use the syntax LIMIT CONNECTIONS so we don't have to add MAX as a keyword at all? I didn't like that either. I was thinking of just CONNECTIONS. LIMIT CONNECTIONS sort of works grammatically, I guess. Would this not work in the context of the general user-specific ALTER USER ... SET something = something? No because it isn't a GUC variable, it is per-user/db value. We could have used that syntax, but it might confuse people. -- 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 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] per user/database connections limit again
An updated version of your patch has been applied and will be in 8.1. Thanks. --- pgman wrote: I have worked over your patch and I think it is ready for application. I changed the syntax to CONNECTION LIMIT, which seems most natural. We could skip CONNECTION and just use a LIMIT keyword, but that seems too terse. I removed your use of the pg_auth flat file. By the time you have the PROC entry to do your lookups, you might as well just use the system cache. There is a race condition in the code because we set our PROC entry before we check for other entries. If there is one connection left and two backends do this at the same time, they would both fail, while one should fail and the other succeed. Without a lock, I see no way to avoid it so I just commented it in the code. Also, I felt that zero should mean allow no/zero connections, rather than representing unlimited connections. I used -1 for unlimited. We can either document the use of -1, or add syntax to allow NO CONNECTION LIMIT, or something like that. The patch requires a catalog version update when applied. -- 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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] per user/database connections limit again
Bruce Momjian pgman@candle.pha.pa.us writes: Peter Eisentraut wrote: Would this not work in the context of the general user-specific ALTER USER ... SET something = something? No because it isn't a GUC variable, it is per-user/db value. We could have used that syntax, but it might confuse people. Yeah --- casting it as a GUC would create issues like what is the global default?. I think treating it as a hard-wired feature is fine. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] per user/database connections limit again
Here is promised documentation.
Be warned that both my writing skills and my english are far from good :)
--
Regards
Petr Jelinek (PJMODOS)
Index: doc/src/sgml/catalogs.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/catalogs.sgml,v
retrieving revision 2.109
diff -c -r2.109 catalogs.sgml
*** doc/src/sgml/catalogs.sgml 26 Jul 2005 16:38:25 - 2.109
--- doc/src/sgml/catalogs.sgml 30 Jul 2005 18:48:10 -
***
*** 1019,1024
--- 1019,1035
/row
row
+ entrystructfieldrolconnlimit/structfield/entry
+ entrytypeint4/type/entry
+ entry/entry
+ entry
+For roles that can login this sets maximum amount of concurrent
+connections this role can make. Default value (-1) means
+unlimited connections, zero (0) means role can't login.
+ /entry
+ /row
+
+ row
entrystructfieldrolpassword/structfield/entry
entrytypetext/type/entry
entry/entry
***
*** 1922,1927
--- 1933,1949
/row
row
+ entrystructfielddatconnlimit/structfield/entry
+ entrytypeint4/type/entry
+ entry/entry
+ entry
+Sets maximum amount of concurrent connections that can be made
+to this database. Default value (-1) means unlimited connections,
+zero (0) means that database isn't accepting connections.
+ /entry
+ /row
+
+ row
entrystructfielddatlastsysoid/structfield/entry
entrytypeoid/type/entry
entry/entry
***
*** 4812,4817
--- 4834,4850
/row
row
+ entrystructfieldrolconnlimit/structfield/entry
+ entrytypeint4/type/entry
+ entry/entry
+ entry
+For roles that can login this sets maximum amount of concurrent
+connections this role can make. Default value (-1) means
+unlimited connections, zero (0) means role can't login.
+ /entry
+ /row
+
+ row
entrystructfieldrolpassword/structfield/entry
entrytypetext/type/entry
entry/entry
***
*** 5094,5099
--- 5127,5143
/row
row
+ entrystructfielduseconnlimit/structfield/entry
+ entrytypeint4/type/entry
+ entry/entry
+ entry
+This sets maximum amount of concurrent connections this user can make.
+Default value (-1) means unlimited connections,
+zero (0) means user can't login.
+ /entry
+ /row
+
+ row
entrystructfieldpasswd/structfield/entry
entrytypetext/type/entry
entry/entry
Index: doc/src/sgml/ref/alter_database.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/alter_database.sgml,v
retrieving revision 1.15
diff -c -r1.15 alter_database.sgml
*** doc/src/sgml/ref/alter_database.sgml5 Jan 2005 14:22:39 -
1.15
--- doc/src/sgml/ref/alter_database.sgml30 Jul 2005 18:48:11 -
***
*** 23,28
--- 23,34
ALTER DATABASE replaceable class=PARAMETERname/replaceable SET
replaceableparameter/replaceable { TO | = } {
replaceablevalue/replaceable | DEFAULT }
ALTER DATABASE replaceable class=PARAMETERname/replaceable RESET
replaceableparameter/replaceable
+ ALTER DATABASE replaceable class=PARAMETERname/replaceable [ [ WITH ]
replaceable class=PARAMETERoption/replaceable [ ... ] ]
+
+ where replaceable class=PARAMETERoption/replaceable can be:
+
+ CONNECTION LIMIT replaceable class=PARAMETERconnlimit/replaceable
+
ALTER DATABASE replaceable class=PARAMETERname/replaceable RENAME TO
replaceablenewname/replaceable
ALTER DATABASE replaceable class=PARAMETERname/replaceable OWNER TO
replaceablenew_owner/replaceable
***
*** 51,57
/para
para
!The third form changes the name of the database. Only the database
owner or a superuser can rename a database; non-superuser owners must
also have the
literalCREATEDB/literal privilege. The current database cannot
--- 57,68
/para
para
!The third form changes certain per-database settings. (See below for
!details.) Only database owner or superuser can change these settings.
! /para
!
! para
!The fourth form changes the name of the database. Only the database
owner or a superuser can rename a database; non-superuser owners must
also have the
literalCREATEDB/literal privilege. The current database cannot
***
*** 60,66
/para
para
!The fourth form changes the owner of the database. Only a superuser
can change the database's owner.
/para
/refsect1
--- 71,77
/para
para
!The fifth form changes the owner of the database. Only a superuser
can change the database's owner.
/para
/refsect1
Re: [PATCHES] per user/database connections limit again
Bruce Momjian pgman@candle.pha.pa.us writes: I have worked over your patch and I think it is ready for application. I've made another pass over this and should be able to commit tomorrow (I'm about to knock off for today, and ran out of time to test pg_dumpall). One thing I changed was that it didn't make sense to me for CREATE DATABASE to copy the template database's datconnlimit. We don't copy its datallowconn or datconfig, so why datconnlimit? BTW I disagree with removing datallowconn; that is different from datconnlimit = 0 because it is enforced even against superusers. (Similar remarks apply for rolcanlogin vs rolconnlimit.) regards, tom lane bin7rNzkZLnar.bin Description: connlimit.patch.gz ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] per user/database connections limit again
Bruce Momjian wrote: I removed your use of the pg_auth flat file. By the time you have the PROC entry to do your lookups, you might as well just use the system cache. There is a race condition in the code because we set our PROC entry before we check for other entries. If there is one connection left and two backends do this at the same time, they would both fail, while one should fail and the other succeed. Without a lock, I see no way to avoid it so I just commented it in the code. Yeah my working version was doing this too but I wanted to avoid lock on PROC array and that race condition and because pg_auth is loaded anyway I used it. Also, I felt that zero should mean allow no/zero connections, rather than representing unlimited connections. I used -1 for unlimited. We can either document the use of -1, or add syntax to allow NO CONNECTION LIMIT, or something like that. Right, maybe we could remove datallowconn from pg_database (in future) if we can achieve same thing using datconnlimit = 0 ? The patch requires a catalog version update when applied. Yes, thanks for your work on this patch, I will write documentation for it in next few days. -- Regards Petr Jelinek (PJMODOS) ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] per user/database connections limit again
Petr Jelinek wrote: Bruce Momjian wrote: I removed your use of the pg_auth flat file. By the time you have the PROC entry to do your lookups, you might as well just use the system cache. There is a race condition in the code because we set our PROC entry before we check for other entries. If there is one connection left and two backends do this at the same time, they would both fail, while one should fail and the other succeed. Without a lock, I see no way to avoid it so I just commented it in the code. Yeah my working version was doing this too but I wanted to avoid lock on PROC array and that race condition and because pg_auth is loaded anyway I used it. Well, we are locking the PROC array for the db scan as well, so I don't see a difference for user. Also, I don't see how it would avoid the race condition. We could scan PROC and then set our user value, but that would allow possibly too many connections rather than too few. Also, I felt that zero should mean allow no/zero connections, rather than representing unlimited connections. I used -1 for unlimited. We can either document the use of -1, or add syntax to allow NO CONNECTION LIMIT, or something like that. Right, maybe we could remove datallowconn from pg_database (in future) if we can achieve same thing using datconnlimit = 0 ? Yes, we certainly could, but I am betting we would need both because if someone wanted to close down a database, _but_ keep the existing limit so it could be resetored later, they would still use datallowconn. The patch requires a catalog version update when applied. Yes, thanks for your work on this patch, I will write documentation for it in next few days. Thanks. -- 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] per user/database connections limit again
The new syntax for this command is CREATE/ALTER DATABASE/USER:
| MAX CONNECTIONS Iconst
This adds 'max' as a keyword, though at a fairly unreserved level, I
think. Should we use the syntax LIMIT CONNECTIONS so we don't have to
add MAX as a keyword at all?
---
Petr Jelinek wrote:
Stephen Frost wrote:
This should almost certainly be a pg_database_ownercheck() call instead.
Right there wasn't pg_database_ownercheck at the time I was writing it,
fixed
The rest needs to be updated for roles, but looks like it should be
pretty easy to do. Much of it just needs to be repatched, the parts
that do need to be changed look to be pretty simple changes.
Done.
I believe the use of SessionUserId is probably correct in this patch.
This does mean that this patch will only be for canlogin roles, but that
seems like it's probably correct. Handling roles w/ members would
require much more thought.
I don't think that having max connection for roles w/ members is doable
because you can have 5 roles which has 1 user as member and each role
has different number of max conections and there is no right way to
decide what to do.
New version which works with roles is attached (diffed against cvs),
everything else is mostly same.
I also had to readd roleid to flatfiles because I need it in
InitProcess() function.
--
Regards
Petr Jelinek (PJMODOS)
Index: src/backend/commands/dbcommands.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/dbcommands.c,v
retrieving revision 1.164
diff -c -r1.164 dbcommands.c
*** src/backend/commands/dbcommands.c 30 Jun 2005 00:00:50 - 1.164
--- src/backend/commands/dbcommands.c 3 Jul 2005 22:47:39 -
***
*** 53,60
/* non-export function prototypes */
static bool get_db_info(const char *name, Oid *dbIdP, Oid *ownerIdP,
! int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
! Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId
*dbFrozenXidP,
Oid *dbTablespace);
static bool have_createdb_privilege(void);
--- 53,60
/* non-export function prototypes */
static bool get_db_info(const char *name, Oid *dbIdP, Oid *ownerIdP,
! int *encodingP, int *dbMaxConnP, bool *dbIsTemplateP,
! bool *dbAllowConnP, Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId
*dbFrozenXidP,
Oid *dbTablespace);
static bool have_createdb_privilege(void);
***
*** 74,79
--- 74,80
int src_encoding;
boolsrc_istemplate;
boolsrc_allowconn;
+ int src_maxconn;
Oid src_lastsysoid;
TransactionId src_vacuumxid;
TransactionId src_frozenxid;
***
*** 91,100
--- 92,103
DefElem*downer = NULL;
DefElem*dtemplate = NULL;
DefElem*dencoding = NULL;
+ DefElem*dmaxconn = NULL;
char *dbname = stmt-dbname;
char *dbowner = NULL;
const char *dbtemplate = NULL;
int encoding = -1;
+ int dbmaxconn = -1;
#ifndef WIN32
charbuf[2 * MAXPGPATH + 100];
***
*** 140,145
--- 143,156
errmsg(conflicting or
redundant options)));
dencoding = defel;
}
+ else if (strcmp(defel-defname, maxconnections) == 0)
+ {
+ if (dmaxconn)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(conflicting or
redundant options)));
+ dmaxconn = defel;
+ }
else if (strcmp(defel-defname, location) == 0)
{
ereport(WARNING,
***
*** 185,190
--- 196,203
elog(ERROR, unrecognized node type: %d,
nodeTag(dencoding-arg));
}
+ if (dmaxconn dmaxconn-arg)
+ dbmaxconn = intVal(dmaxconn-arg);
/* obtain OID of proposed owner */
if (dbowner)
***
*** 218,224
* idea, so accept possibility of race to create. We will check again
* after we grab the exclusive lock.
*/
! if (get_db_info(dbname, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL))
ereport(ERROR,
Re: [PATCHES] per user/database connections limit again
Bruce Momjian pgman@candle.pha.pa.us writes: The new syntax for this command is CREATE/ALTER DATABASE/USER: | MAX CONNECTIONS Iconst This adds 'max' as a keyword, though at a fairly unreserved level, I think. Should we use the syntax LIMIT CONNECTIONS so we don't have to add MAX as a keyword at all? I didn't like that either. I was thinking of just CONNECTIONS. LIMIT CONNECTIONS sort of works grammatically, I guess. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] per user/database connections limit again
Bruce Momjian wrote: The new syntax for this command is CREATE/ALTER DATABASE/USER: | MAX CONNECTIONS Iconst This adds 'max' as a keyword, though at a fairly unreserved level, I think. Should we use the syntax LIMIT CONNECTIONS so we don't have to add MAX as a keyword at all? Yeah I have no problem with LIMIT CONNECTIONS, will you change it or should I do it ? btw where has new keyword to be added to not be added at a fairly unreserved level ? (MAX is also added to keywords.c in that patch) -- Regards Petr Jelinek (PJMODOS) ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] per user/database connections limit again
Petr Jelinek wrote: Bruce Momjian wrote: The new syntax for this command is CREATE/ALTER DATABASE/USER: | MAX CONNECTIONS Iconst This adds 'max' as a keyword, though at a fairly unreserved level, I think. Should we use the syntax LIMIT CONNECTIONS so we don't have to add MAX as a keyword at all? Yeah I have no problem with LIMIT CONNECTIONS, will you change it or should I do it ? I will do it. btw where has new keyword to be added to not be added at a fairly unreserved level ? (MAX is also added to keywords.c in that patch) Right, I will remove the MAX addition. parser/gram.y has this comment: /* * Keyword classification lists. Generally, every keyword present in * the Postgres grammar should appear in exactly one of these lists. * * Put a new keyword into the first list that it can go into without causing * shift or reduce conflicts. The earlier lists define less reserved * categories of keywords. */ I will check that your additions are in the right place. -- 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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] per user/database connections limit again
Bruce Momjian napsal(a): I am ready to apply this patch. Would you make the additional changes you suggested? Is there any way to see the limits except to query pg_authid? Yes I will - pg_dump is already done (I attached it because it should be aplied with orginal patch), documentation depends partly on roles doc so it will prolly have to wait. I also added limit to pg_roles and pg_shadow views when I was patching pg_dump so you can get it from them. -- Regards Petr Jelinek (PJMODOS) Index: src/backend/catalog/system_views.sql === RCS file: /projects/cvsroot/pgsql/src/backend/catalog/system_views.sql,v retrieving revision 1.16 diff -c -r1.16 system_views.sql *** src/backend/catalog/system_views.sql28 Jun 2005 05:08:52 - 1.16 --- src/backend/catalog/system_views.sql24 Jul 2005 12:22:08 - *** *** 14,19 --- 14,20 rolcreatedb, rolcatupdate, rolcanlogin, + rolmaxconn, ''::text as rolpassword, rolvaliduntil, rolconfig *** *** 26,31 --- 27,33 rolcreatedb AS usecreatedb, rolsuper AS usesuper, rolcatupdate AS usecatupd, + rolmaxconn AS usemaxconn, rolpassword AS passwd, rolvaliduntil::abstime AS valuntil, rolconfig AS useconfig Index: src/bin/pg_dump/pg_dumpall.c === RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_dumpall.c,v retrieving revision 1.64 diff -c -r1.64 pg_dumpall.c *** src/bin/pg_dump/pg_dumpall.c18 Jul 2005 19:12:09 - 1.64 --- src/bin/pg_dump/pg_dumpall.c24 Jul 2005 12:22:35 - *** *** 394,409 PGresult *res; int i; ! if (server_version = 70100) res = executeQuery(conn, SELECT usename, usesysid, passwd, usecreatedb, ! usesuper, valuntil, (usesysid = (SELECT datdba FROM pg_database WHERE datname = 'template0')) AS clusterowner FROM pg_shadow); else res = executeQuery(conn, SELECT usename, usesysid, passwd, usecreatedb, ! usesuper, valuntil, (usesysid = (SELECT datdba FROM pg_database WHERE datname = 'template1')) AS clusterowner FROM pg_shadow); --- 394,415 PGresult *res; int i; ! if (server_version = 80100) ! res = executeQuery(conn, ! SELECT usename, usesysid, passwd, usecreatedb, ! usesuper, valuntil, usemaxconn, ! (usesysid = (SELECT datdba FROM pg_database WHERE datname = 'template0')) AS clusterowner ! FROM pg_shadow); ! else if (server_version = 70100) res = executeQuery(conn, SELECT usename, usesysid, passwd, usecreatedb, ! usesuper, valuntil, '0' AS usemaxconn, (usesysid = (SELECT datdba FROM pg_database WHERE datname = 'template0')) AS clusterowner FROM pg_shadow); else res = executeQuery(conn, SELECT usename, usesysid, passwd, usecreatedb, ! usesuper, valuntil, '0' AS usemaxconn, (usesysid = (SELECT datdba FROM pg_database WHERE datname = 'template1')) AS clusterowner FROM pg_shadow); *** *** 453,458 --- 459,468 appendPQExpBuffer(buf, VALID UNTIL '%s', PQgetvalue(res, i, 5)); + if (strcmp(PQgetvalue(res, i, 6), 0) != 0) + appendPQExpBuffer(buf, MAX CONNECTIONS '%s', + PQgetvalue(res, i, 6)); + appendPQExpBuffer(buf, ;\n); printf(%s, buf-data); *** *** 612,623 printf(--\n-- Database creation\n--\n\n); ! if (server_version = 8) res = executeQuery(conn,
Re: [PATCHES] per user/database connections limit again
Alvaro Herrera wrote: I was wondering if there was some way to defer the user check till a later time, when the pg_authid relation could be checked? Not sure if that's a good idea, but it may help reduce the impact of the change, and thus chances that it'd be rejected. Well It can, but it would mean one more lock on procarray and I didn't want that and like I said, MyDatabaseId is read from flatfile too. Auth flatfile is used only on two other places which I also patched so I don't see this as a problem (it's used in hba.c to check role membership and in crypt.c for password verification) -- Regards Petr Jelinek (PJMODOS) ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] per user/database connections limit again
Stephen Frost wrote:
This should almost certainly be a pg_database_ownercheck() call instead.
Right there wasn't pg_database_ownercheck at the time I was writing it,
fixed
The rest needs to be updated for roles, but looks like it should be
pretty easy to do. Much of it just needs to be repatched, the parts
that do need to be changed look to be pretty simple changes.
Done.
I believe the use of SessionUserId is probably correct in this patch.
This does mean that this patch will only be for canlogin roles, but that
seems like it's probably correct. Handling roles w/ members would
require much more thought.
I don't think that having max connection for roles w/ members is doable
because you can have 5 roles which has 1 user as member and each role
has different number of max conections and there is no right way to
decide what to do.
New version which works with roles is attached (diffed against cvs),
everything else is mostly same.
I also had to readd roleid to flatfiles because I need it in
InitProcess() function.
--
Regards
Petr Jelinek (PJMODOS)
Index: src/backend/commands/dbcommands.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/dbcommands.c,v
retrieving revision 1.164
diff -c -r1.164 dbcommands.c
*** src/backend/commands/dbcommands.c 30 Jun 2005 00:00:50 - 1.164
--- src/backend/commands/dbcommands.c 3 Jul 2005 22:47:39 -
***
*** 53,60
/* non-export function prototypes */
static bool get_db_info(const char *name, Oid *dbIdP, Oid *ownerIdP,
! int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
! Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId
*dbFrozenXidP,
Oid *dbTablespace);
static bool have_createdb_privilege(void);
--- 53,60
/* non-export function prototypes */
static bool get_db_info(const char *name, Oid *dbIdP, Oid *ownerIdP,
! int *encodingP, int *dbMaxConnP, bool *dbIsTemplateP,
! bool *dbAllowConnP, Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId
*dbFrozenXidP,
Oid *dbTablespace);
static bool have_createdb_privilege(void);
***
*** 74,79
--- 74,80
int src_encoding;
boolsrc_istemplate;
boolsrc_allowconn;
+ int src_maxconn;
Oid src_lastsysoid;
TransactionId src_vacuumxid;
TransactionId src_frozenxid;
***
*** 91,100
--- 92,103
DefElem*downer = NULL;
DefElem*dtemplate = NULL;
DefElem*dencoding = NULL;
+ DefElem*dmaxconn = NULL;
char *dbname = stmt-dbname;
char *dbowner = NULL;
const char *dbtemplate = NULL;
int encoding = -1;
+ int dbmaxconn = -1;
#ifndef WIN32
charbuf[2 * MAXPGPATH + 100];
***
*** 140,145
--- 143,156
errmsg(conflicting or
redundant options)));
dencoding = defel;
}
+ else if (strcmp(defel-defname, maxconnections) == 0)
+ {
+ if (dmaxconn)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+errmsg(conflicting or
redundant options)));
+ dmaxconn = defel;
+ }
else if (strcmp(defel-defname, location) == 0)
{
ereport(WARNING,
***
*** 185,190
--- 196,203
elog(ERROR, unrecognized node type: %d,
nodeTag(dencoding-arg));
}
+ if (dmaxconn dmaxconn-arg)
+ dbmaxconn = intVal(dmaxconn-arg);
/* obtain OID of proposed owner */
if (dbowner)
***
*** 218,224
* idea, so accept possibility of race to create. We will check again
* after we grab the exclusive lock.
*/
! if (get_db_info(dbname, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_DATABASE),
--- 231,237
* idea, so accept possibility of race to create. We will check again
* after we grab the exclusive lock.
*/
! if (get_db_info(dbname, NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL))
ereport(ERROR,
Re: [PATCHES] per user/database connections limit again
On Mon, Jul 04, 2005 at 01:08:05AM +0200, Petr Jelinek wrote: Stephen Frost wrote: New version which works with roles is attached (diffed against cvs), everything else is mostly same. I also had to readd roleid to flatfiles because I need it in InitProcess() function. I was wondering if there was some way to defer the user check till a later time, when the pg_authid relation could be checked? Not sure if that's a good idea, but it may help reduce the impact of the change, and thus chances that it'd be rejected. -- Alvaro Herrera (alvherre[a]surnet.cl) La espina, desde que nace, ya pincha (Proverbio africano) ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] per user/database connections limit again
We will need these:
Patch includes only changes to backend, I will make pg_dump, ecpg and
documentation patches once this is completed and accepted by team.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---
Petr Jelinek wrote:
Hi,
I attached second try of per-database and per-user connection limit for
your review.
This time I am using information stored in ProcArray to get number of
connections - I modified PGPROC struct to also include userid.
Limits for user and database are stored in catalog tables. This aproach
led to implementation of universal ALTER DATABASE query (I followed
ALTER USER ad ALTER DATABASE ... RENAME implementatons). So queries for
setting maximum connections look like this: CREATE|ALTER DATABASE|USER
name MAX CONNECTIONS = 20;
Maximum connections defaults to zero which means unlimited (limited by
global maximum only) and isn't enforced for superusers.
The actual check for maximum conenctions is done in ReverifyMyDatabase
for database and InitializeSessionUser for user because we don't have
information from system catalog before so we don't know how many
connections are allowed.
Patch includes only changes to backend, I will make pg_dump, ecpg and
documentation patches once this is completed and accepted by team.
Diff is made against cvs from today morning GMT (apply with -p1 if you
want to test it) - cvs is down now so I can't make diff against repository.
--
Regards
Petr Jelinek (PJMODOS)
diff -Nacr my-cvs/src/backend/commands/dbcommands.c
my-aproach2/src/backend/commands/dbcommands.c
*** my-cvs/src/backend/commands/dbcommands.c Sun Jun 26 00:47:30 2005
--- my-aproach2/src/backend/commands/dbcommands.c Tue Jun 28 11:26:08 2005
***
*** 53,60
/* non-export function prototypes */
static bool get_db_info(const char *name, Oid *dbIdP, int4 *ownerIdP,
! int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
! Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId
*dbFrozenXidP,
Oid *dbTablespace);
static bool have_createdb_privilege(void);
--- 53,60
/* non-export function prototypes */
static bool get_db_info(const char *name, Oid *dbIdP, int4 *ownerIdP,
! int *encodingP, int *dbMaxConnP, bool *dbIsTemplateP,
! bool *dbAllowConnP, Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId
*dbFrozenXidP,
Oid *dbTablespace);
static bool have_createdb_privilege(void);
***
*** 74,79
--- 74,80
int src_encoding;
boolsrc_istemplate;
boolsrc_allowconn;
+ int src_maxconn;
Oid src_lastsysoid;
TransactionId src_vacuumxid;
TransactionId src_frozenxid;
***
*** 91,100
--- 92,103
DefElem*downer = NULL;
DefElem*dtemplate = NULL;
DefElem*dencoding = NULL;
+ DefElem*dmaxconn = NULL;
char *dbname = stmt-dbname;
char *dbowner = NULL;
const char *dbtemplate = NULL;
int encoding = -1;
+ int dbmaxconn = -1;
#ifndef WIN32
charbuf[2 * MAXPGPATH + 100];
***
*** 140,145
--- 143,156
errmsg(conflicting or
redundant options)));
dencoding = defel;
}
+ else if (strcmp(defel-defname, maxconnections) == 0)
+ {
+ if (dmaxconn)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(conflicting or
redundant options)));
+ dmaxconn = defel;
+ }
else if (strcmp(defel-defname, location) == 0)
{
ereport(WARNING,
***
*** 185,190
--- 196,203
elog(ERROR, unrecognized node type: %d,
nodeTag(dencoding-arg));
}
+ if (dmaxconn dmaxconn-arg)
+ dbmaxconn = intVal(dmaxconn-arg);
/* obtain sysid of proposed owner */
if (dbowner)
***
*** 218,224
* idea, so accept possibility of race to create. We will check again
* after we grab the exclusive lock.
*/
! if (get_db_info(dbname, NULL, NULL, NULL,
Re: [PATCHES] per user/database connections limit again
On Sat, Jul 02, 2005 at 04:28:48PM -0400, Bruce Momjian wrote: We will need these: Patch includes only changes to backend, I will make pg_dump, ecpg and documentation patches once this is completed and accepted by team. Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. It needs to be updated to account for the roles patch. -- Alvaro Herrera (alvherre[a]surnet.cl) El Maquinismo fue proscrito so pena de cosquilleo hasta la muerte (Ijon Tichy en Viajes, Stanislaw Lem) ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] per user/database connections limit again
Alvaro Herrera wrote: On Sat, Jul 02, 2005 at 04:28:48PM -0400, Bruce Momjian wrote: We will need these: Patch includes only changes to backend, I will make pg_dump, ecpg and documentation patches once this is completed and accepted by team. Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. It needs to be updated to account for the roles patch. I was afraid of that. :-( Thanks. -- 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 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] per user/database connections limit again
* Petr Jelinek ([EMAIL PROTECTED]) wrote: + if (!(superuser() + || ((Form_pg_database) GETSTRUCT(tuple))-datdba == GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_DATABASE, +stmt-dbname); This should almost certainly be a pg_database_ownercheck() call instead. The rest needs to be updated for roles, but looks like it should be pretty easy to do. Much of it just needs to be repatched, the parts that do need to be changed look to be pretty simple changes. I believe the use of SessionUserId is probably correct in this patch. This does mean that this patch will only be for canlogin roles, but that seems like it's probably correct. Handling roles w/ members would require much more thought. Thanks, Stephen signature.asc Description: Digital signature
[PATCHES] per user/database connections limit again
Hi,
I attached second try of per-database and per-user connection limit for
your review.
This time I am using information stored in ProcArray to get number of
connections - I modified PGPROC struct to also include userid.
Limits for user and database are stored in catalog tables. This aproach
led to implementation of universal ALTER DATABASE query (I followed
ALTER USER ad ALTER DATABASE ... RENAME implementatons). So queries for
setting maximum connections look like this: CREATE|ALTER DATABASE|USER
name MAX CONNECTIONS = 20;
Maximum connections defaults to zero which means unlimited (limited by
global maximum only) and isn't enforced for superusers.
The actual check for maximum conenctions is done in ReverifyMyDatabase
for database and InitializeSessionUser for user because we don't have
information from system catalog before so we don't know how many
connections are allowed.
Patch includes only changes to backend, I will make pg_dump, ecpg and
documentation patches once this is completed and accepted by team.
Diff is made against cvs from today morning GMT (apply with -p1 if you
want to test it) - cvs is down now so I can't make diff against repository.
--
Regards
Petr Jelinek (PJMODOS)
diff -Nacr my-cvs/src/backend/commands/dbcommands.c
my-aproach2/src/backend/commands/dbcommands.c
*** my-cvs/src/backend/commands/dbcommands.cSun Jun 26 00:47:30 2005
--- my-aproach2/src/backend/commands/dbcommands.c Tue Jun 28 11:26:08 2005
***
*** 53,60
/* non-export function prototypes */
static bool get_db_info(const char *name, Oid *dbIdP, int4 *ownerIdP,
! int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
! Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId
*dbFrozenXidP,
Oid *dbTablespace);
static bool have_createdb_privilege(void);
--- 53,60
/* non-export function prototypes */
static bool get_db_info(const char *name, Oid *dbIdP, int4 *ownerIdP,
! int *encodingP, int *dbMaxConnP, bool *dbIsTemplateP,
! bool *dbAllowConnP, Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId
*dbFrozenXidP,
Oid *dbTablespace);
static bool have_createdb_privilege(void);
***
*** 74,79
--- 74,80
int src_encoding;
boolsrc_istemplate;
boolsrc_allowconn;
+ int src_maxconn;
Oid src_lastsysoid;
TransactionId src_vacuumxid;
TransactionId src_frozenxid;
***
*** 91,100
--- 92,103
DefElem*downer = NULL;
DefElem*dtemplate = NULL;
DefElem*dencoding = NULL;
+ DefElem*dmaxconn = NULL;
char *dbname = stmt-dbname;
char *dbowner = NULL;
const char *dbtemplate = NULL;
int encoding = -1;
+ int dbmaxconn = -1;
#ifndef WIN32
charbuf[2 * MAXPGPATH + 100];
***
*** 140,145
--- 143,156
errmsg(conflicting or
redundant options)));
dencoding = defel;
}
+ else if (strcmp(defel-defname, maxconnections) == 0)
+ {
+ if (dmaxconn)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+errmsg(conflicting or
redundant options)));
+ dmaxconn = defel;
+ }
else if (strcmp(defel-defname, location) == 0)
{
ereport(WARNING,
***
*** 185,190
--- 196,203
elog(ERROR, unrecognized node type: %d,
nodeTag(dencoding-arg));
}
+ if (dmaxconn dmaxconn-arg)
+ dbmaxconn = intVal(dmaxconn-arg);
/* obtain sysid of proposed owner */
if (dbowner)
***
*** 218,224
* idea, so accept possibility of race to create. We will check again
* after we grab the exclusive lock.
*/
! if (get_db_info(dbname, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_DATABASE),
--- 231,237
* idea, so accept possibility of race to create. We will check again
* after we grab the exclusive lock.
*/
! if (get_db_info(dbname, NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL))
ereport(ERROR,
