Thread: advisory locks and permissions
One good thing about advisory locks having been in contrib was that they
didn't affect anyone who didn't actually install the module. Now that
we've put those functions in core, I wonder whether we don't need to
face up to the possibility of malicious use. For instance, it's not
very hard to create a DoS situation by running the system out of shared
lock table space:
regression=# select pg_advisory_lock(x) from generate_series(1,1000000) x;
WARNING: out of shared memory
ERROR: out of shared memory
HINT: You may need to increase max_locks_per_transaction.
regression=#
and once you've done that about the only fix is to quit your backend :-(
The brute force answer is to make those functions superuser-only, but I
wonder if there is a better way. Perhaps we could just deny public
execute access on them by default, and let admins grant the privilege to
whom they trust.
Or we could try to do something about limiting the number of such locks
that can be granted, but that seems nontrivial to tackle at such a late
stage of the devel cycle.
Thoughts?
regards, tom lane
On Wed, Sep 20, 2006 at 07:52:33PM -0400, Tom Lane wrote: > face up to the possibility of malicious use. For instance, it's not > very hard to create a DoS situation by running the system out of shared > lock table space: Didn't you just say we don't try and protect against DoS? ;P > The brute force answer is to make those functions superuser-only, but I > wonder if there is a better way. Perhaps we could just deny public > execute access on them by default, and let admins grant the privilege to > whom they trust. > > Or we could try to do something about limiting the number of such locks > that can be granted, but that seems nontrivial to tackle at such a late > stage of the devel cycle. ISTM that just restricting default access still leaves a pretty big foot-gun laying around... perhaps the best compromise would be to do that for this release and add some kind of a limit in the next release. -- Jim Nasby jim@nasby.net EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On 9/21/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > One good thing about advisory locks having been in contrib was that they > didn't affect anyone who didn't actually install the module. Now that > we've put those functions in core, I wonder whether we don't need to > face up to the possibility of malicious use. For instance, it's not > very hard to create a DoS situation by running the system out of shared > lock table space: It's much more likely to accidentally bork yourself. > regression=# select pg_advisory_lock(x) from generate_series(1,1000000) x; > WARNING: out of shared memory > ERROR: out of shared memory > HINT: You may need to increase max_locks_per_transaction. > regression=# > > and once you've done that about the only fix is to quit your backend :-( > > The brute force answer is to make those functions superuser-only, but I > wonder if there is a better way. Perhaps we could just deny public > execute access on them by default, and let admins grant the privilege to > whom they trust. > > Or we could try to do something about limiting the number of such locks > that can be granted, but that seems nontrivial to tackle at such a late > stage of the devel cycle. I vote for locking down to superuser access (lets be frank here: I would estimate 90%+ database installatons run with the application as root) so we are not losing much. I honestly think exhausting the lock space should raise a fatal, not an error. This gives a chance for recovery without a hard reset. Any other limitation would be fine, now or later, just please don't slow them down! (advisory locks are extremely fast). merlin
All, > I vote for locking down to superuser access (lets be frank here: I > would estimate 90%+ database installatons run with the application as > root) so we are not losing much. Not in my experience. Note that making them superuser-only pretty much puts them out of the hands of hosted applications. How simple would it be to limit the number of advisory locks available to a single request? That would at least make the DOS non-trivial. Or to put in a handle (GUC?) that allows turning advisory locks off? Hmmm ... I'll bet I could come up with other ways to use generate_series in a DOS, even without advisory locks ... -- Josh Berkus PostgreSQL @ Sun San Francisco
Doesn't creating many temp tables in a transaction do the same thing? --------------------------------------------------------------------------- Josh Berkus wrote: > All, > > > I vote for locking down to superuser access (lets be frank here: I > > would estimate 90%+ database installatons run with the application as > > root) so we are not losing much. > > Not in my experience. Note that making them superuser-only pretty much puts > them out of the hands of hosted applications. > > How simple would it be to limit the number of advisory locks available to a > single request? That would at least make the DOS non-trivial. Or to put in > a handle (GUC?) that allows turning advisory locks off? > > Hmmm ... I'll bet I could come up with other ways to use generate_series in a > DOS, even without advisory locks ... > > -- > Josh Berkus > PostgreSQL @ Sun > San Francisco > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Wed, 20 Sep 2006, Bruce Momjian wrote: > > Doesn't creating many temp tables in a transaction do the same thing? > > --------------------------------------------------------------------------- Like this? jeremyd=# CREATE OR REPLACE FUNCTION testy(n integer) returns integer as $$ BEGINEXECUTE 'CREATE TEMP TABLE testy_' || n::text || ' (a integer, b text);';RETURN n; END; $$ LANGUAGE plpgsql VOLATILE STRICT; CREATE FUNCTION jeremyd=# select testy(n) from generate_series(1,1000000) n; WARNING: out of shared memory CONTEXT: SQL statement "CREATE TEMP TABLE testy_3323 (a integer, b text);" PL/pgSQL function "testy" line 2 at execute statement ERROR: out of shared memory HINT: You may need to increase max_locks_per_transaction. CONTEXT: SQL statement "CREATE TEMP TABLE testy_3323 (a integer, b text);" PL/pgSQL function "testy" line 2 at execute statement > > Josh Berkus wrote: > > All, > > > > > I vote for locking down to superuser access (lets be frank here: I > > > would estimate 90%+ database installatons run with the application as > > > root) so we are not losing much. > > > > Not in my experience. Note that making them superuser-only pretty much puts > > them out of the hands of hosted applications. > > > > How simple would it be to limit the number of advisory locks available to a > > single request? That would at least make the DOS non-trivial. Or to put in > > a handle (GUC?) that allows turning advisory locks off? > > > > Hmmm ... I'll bet I could come up with other ways to use generate_series in a > > DOS, even without advisory locks ... > > > > -- > > Josh Berkus > > PostgreSQL @ Sun > > San Francisco > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 6: explain analyze is your friend > > -- Two percent of zero is almost nothing.
Bruce Momjian <bruce@momjian.us> writes:
> Doesn't creating many temp tables in a transaction do the same thing?
True, but it's a tad harder/less likely that you'd accidentally cause
a problem that way.
I'm not sure if I'm crying wolf or whether there's a serious issue.
Certainly, if you have SQL-command access there are plenty of ways
to cause DoS situations of varying levels of severity.
An admin who is concerned about this can revoke public access on the
functions for himself ... but should that be the default out-of-the-box
configuration? I feel more comfortable with saying "you have to turn
on this potentially-dangerous feature" than with saying you have to turn
it off.
Another reason for restricting access to the advisory-lock functions
is that an uninformed application might take the wrong locks, and
bollix up your intended usage accidentally.
regards, tom lane
Le jeudi 21 septembre 2006 01:52, Tom Lane a écrit : > Or we could try to do something about limiting the number of such locks > that can be granted, but that seems nontrivial to tackle at such a late > stage of the devel cycle. > > Thoughts? What about reserving some amount of shared_buffers out of those locks? (For example ext2 preserve some disk space for root in case of emergency) Don't know anything about how easily (error prone) this can be done, though. Le jeudi 21 septembre 2006 16:22, Tom Lane a écrit : > Another reason for restricting access to the advisory-lock functions > is that an uninformed application might take the wrong locks, and > bollix up your intended usage accidentally. This sounds like one more attempt to protect against idiots, which universe tend to produce on a pretty quick rate :) My 2¢, -- Dimitri Fontaine Directeur Technique Tel: 06 74 15 56 53
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> An admin who is concerned about this can revoke public access on the
> functions for himself ... but should that be the default out-of-the-box
> configuration? I feel more comfortable with saying "you have to turn
> on this potentially-dangerous feature" than with saying you have to turn
> it off.
I agree with having it turned off by default, at least in 8.2. Once
it's out there and in use we can review that decision for 8.3 and
possibly implement appropriate safeguards based on the usage.
> Another reason for restricting access to the advisory-lock functions
> is that an uninformed application might take the wrong locks, and
> bollix up your intended usage accidentally.
This begs for a mechanism to define who can take what locks, etc..
Which seems to be an 8.3 issue.
Thanks,
Stephen
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Doesn't creating many temp tables in a transaction do the same thing? > > True, but it's a tad harder/less likely that you'd accidentally cause > a problem that way. Then why not use a GUC (that only an administrator can set) to control the maximum number of advisory locks a given backend can take at any one time? Then it becomes the DBA's problem (and solution) if someone manages to run the database out of shared memory through this mechanism. -- Kevin Brown kevin@sysexperts.com
Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> An admin who is concerned about this can revoke public access on the
>> functions for himself ... but should that be the default out-of-the-box
>> configuration? I feel more comfortable with saying "you have to turn
>> on this potentially-dangerous feature" than with saying you have to turn
>> it off.
> I agree with having it turned off by default, at least in 8.2.
Do we have a consensus to do this for 8.2? Or are we going to leave it
as is? Those are the only two realistic short-term options ...
regards, tom lane
On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Stephen Frost <sfrost@snowman.net> writes: > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >> An admin who is concerned about this can revoke public access on the > >> functions for himself ... but should that be the default out-of-the-box > >> configuration? I feel more comfortable with saying "you have to turn > >> on this potentially-dangerous feature" than with saying you have to turn > >> it off. > > > I agree with having it turned off by default, at least in 8.2. > > Do we have a consensus to do this for 8.2? Or are we going to leave it > as is? Those are the only two realistic short-term options ... there are plenty of other potentially nasty things (like generate_series and the ! operator). why are advisory_locks handled specially? the way it stands right now is a user with command access can DoS a server after five minutes of research on the web. however, if we decide to lock them, it should be documented as such. advisory locks still show up as 'userlock' in the pg_locks view. does this matter? merlin
"Merlin Moncure" <mmoncure@gmail.com> writes:
> advisory locks still show up as 'userlock' in the pg_locks view. does
> this matter?
I'm disinclined to change that, because it would probably break existing
client-side code for little gain.
regards, tom lane
> there are plenty of other potentially nasty things (like > generate_series and the ! operator). why are advisory_locks handled > specially? the way it stands right now is a user with command access > can DoS a server after five minutes of research on the web. You don't even have to do any research, just fire off ab. Using a DOS to attack *any* database server via the web is a 3 second command. Joshua D. Drake -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240 Providing the most comprehensive PostgreSQL solutionssince 1997 http://www.commandprompt.com/
On Sep 22, 2006, at 11:26 , Merlin Moncure wrote: > On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Stephen Frost <sfrost@snowman.net> writes: >> > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> >> An admin who is concerned about this can revoke public access >> on the >> >> functions for himself ... but should that be the default out-of- >> the-box >> >> configuration? I feel more comfortable with saying "you have >> to turn >> >> on this potentially-dangerous feature" than with saying you >> have to turn >> >> it off. >> >> > I agree with having it turned off by default, at least in 8.2. >> >> Do we have a consensus to do this for 8.2? Or are we going to >> leave it >> as is? Those are the only two realistic short-term options ... > > there are plenty of other potentially nasty things (like > generate_series and the ! operator). why are advisory_locks handled > specially? the way it stands right now is a user with command access > can DoS a server after five minutes of research on the web. > > however, if we decide to lock them, it should be documented as such. > > advisory locks still show up as 'userlock' in the pg_locks view. does > this matter? I would be more worried about accidental collisions between applications. The lock ranges will now need to be in their respective application's configuration file in case of collision with another app once developers really start using locks for IPC. Ideally, the user-level lock functions would take strings instead of integers and hash them appropriately, no? Otherwise, someone will end up maintaining a registry of lock numbers in use. LISTEN doesn't use integers. -M
On Fri, Sep 22, 2006 at 12:03:46PM -0400, AgentM wrote: > > On Sep 22, 2006, at 11:26 , Merlin Moncure wrote: > > >On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >>Stephen Frost <sfrost@snowman.net> writes: > >>> * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >>>> An admin who is concerned about this can revoke public access > >>on the > >>>> functions for himself ... but should that be the default out-of- > >>the-box > >>>> configuration? I feel more comfortable with saying "you have > >>to turn > >>>> on this potentially-dangerous feature" than with saying you > >>have to turn > >>>> it off. > >> > >>> I agree with having it turned off by default, at least in 8.2. > >> > >>Do we have a consensus to do this for 8.2? Or are we going to > >>leave it > >>as is? Those are the only two realistic short-term options ... > > > >there are plenty of other potentially nasty things (like > >generate_series and the ! operator). why are advisory_locks handled > >specially? the way it stands right now is a user with command access > >can DoS a server after five minutes of research on the web. > > > >however, if we decide to lock them, it should be documented as such. > > > >advisory locks still show up as 'userlock' in the pg_locks view. does > >this matter? > > I would be more worried about accidental collisions between > applications. The lock ranges will now need to be in their respective > application's configuration file in case of collision with another > app once developers really start using locks for IPC. Ideally, the > user-level lock functions would take strings instead of integers and > hash them appropriately, no? Otherwise, someone will end up > maintaining a registry of lock numbers in use. LISTEN doesn't use > integers. This is why I suggested we set aside some range of numbers that should not be used. Doing so would allow adding a better-managed numbering/naming scheme in the future. -- Jim Nasby jim@nasby.net EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On 9/22/06, AgentM <agentm@themactionfaction.com> wrote: > I would be more worried about accidental collisions between > applications. The lock ranges will now need to be in their respective i dont think this argument has merit because the lock is scoped to the current database. this would only be a problem if two applications used the same database...not likely. the old userlocks had 48 bits of lock space and now we have 64, im not complaining. > application's configuration file in case of collision with another > app once developers really start using locks for IPC. Ideally, the > user-level lock functions would take strings instead of integers and > hash them appropriately, no? Otherwise, someone will end up > maintaining a registry of lock numbers in use. LISTEN doesn't use > integers. application can translate the locks to strings via a very simple translation table. there is no downside to this besides a index lookup on a small table, which is more or less what the listen/notify does internally. advisory locks work off of the internal lock system which is an integer only system. the whole point is to get at these locks while bypassing the transaction system. you are suggesting something which does not fit into the current lock system. merlin
On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote: > This is why I suggested we set aside some range of numbers that should > not be used. Doing so would allow adding a better-managed > numbering/naming scheme in the future. the whole point about advisory locks is that the provided lock space is unmanaged. for example, in the ISAM system I wrote which hooked into the acucobol virtual file system interface, I used a global sequence for row level pessimistic locking but reserved the 48th bit for table level locks. This system was extremely effective. on the current system I'm working on I use them to lock sequence oid's plus a high bit indicator for what i am doing. in short, advisory locks are application-defined in concept. merlin
On Sep 22, 2006, at 12:46 , Merlin Moncure wrote: > On 9/22/06, AgentM <agentm@themactionfaction.com> wrote: >> I would be more worried about accidental collisions between >> applications. The lock ranges will now need to be in their respective > > i dont think this argument has merit because the lock is scoped to the > current database. this would only be a problem if two applications > used the same database...not likely. Since we have schemas, I have several applications running in one database- sometimes several versions of the same application. This makes it easier to shuffle data around. >> application's configuration file in case of collision with another >> app once developers really start using locks for IPC. Ideally, the >> user-level lock functions would take strings instead of integers and >> hash them appropriately, no? Otherwise, someone will end up >> maintaining a registry of lock numbers in use. LISTEN doesn't use >> integers. > > application can translate the locks to strings via a very simple > translation table. there is no downside to this besides a index > lookup on a small table, which is more or less what the listen/notify > does internally. > > advisory locks work off of the internal lock system which is an > integer only system. the whole point is to get at these locks while > bypassing the transaction system. you are suggesting something which > does not fit into the current lock system. I didn't suggest using lookup tables; I suggested that the lock functions should perform the string hashing itself- the applications will write wrappers for this anyway to prevent collisions or they will have to provide some configuration element to change the lock range in case of collision- which will be extraordinarily difficult to debug in the first place. -M
On Fri, Sep 22, 2006 at 12:56:37PM -0400, Merlin Moncure wrote: > On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote: > >This is why I suggested we set aside some range of numbers that should > >not be used. Doing so would allow adding a better-managed > >numbering/naming scheme in the future. > > the whole point about advisory locks is that the provided lock space > is unmanaged. for example, in the ISAM system I wrote which hooked > into the acucobol virtual file system interface, I used a global > sequence for row level pessimistic locking but reserved the 48th bit > for table level locks. This system was extremely effective. on the > current system I'm working on I use them to lock sequence oid's plus a > high bit indicator for what i am doing. in short, advisory locks are > application-defined in concept. Yes, but if you get two pieces of code written by different people using them in the same database, you can get hosed. As PostgreSQL becomes more popular and more people start developing software for it, this is more likely to occur. -- Jim Nasby jim@nasby.net EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote: > On Fri, Sep 22, 2006 at 12:56:37PM -0400, Merlin Moncure wrote: > > the whole point about advisory locks is that the provided lock space > > is unmanaged. for example, in the ISAM system I wrote which hooked > > into the acucobol virtual file system interface, I used a global > > sequence for row level pessimistic locking but reserved the 48th bit > > for table level locks. This system was extremely effective. on the > > current system I'm working on I use them to lock sequence oid's plus a > > high bit indicator for what i am doing. in short, advisory locks are > > application-defined in concept. > > Yes, but if you get two pieces of code written by different people using > them in the same database, you can get hosed. As PostgreSQL becomes more > popular and more people start developing software for it, this is more > likely to occur. imo, that is no more or less likely than having two pieces of code store the same table in the same database. I think what you are describing would only be a concern if the locks were shared across databases, however this is not the case. the purpose of advisory locks is to be 'appplication-defined'. how the application is written is not part of that concept. we are simply granting the ability to create a mutex with a number for a name, that is all. merlin
On Fri, Sep 22, 2006 at 01:21:57PM -0400, Merlin Moncure wrote: > On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote: > >On Fri, Sep 22, 2006 at 12:56:37PM -0400, Merlin Moncure wrote: > >> the whole point about advisory locks is that the provided lock space > >> is unmanaged. for example, in the ISAM system I wrote which hooked > >> into the acucobol virtual file system interface, I used a global > >> sequence for row level pessimistic locking but reserved the 48th bit > >> for table level locks. This system was extremely effective. on the > >> current system I'm working on I use them to lock sequence oid's plus a > >> high bit indicator for what i am doing. in short, advisory locks are > >> application-defined in concept. > > > >Yes, but if you get two pieces of code written by different people using > >them in the same database, you can get hosed. As PostgreSQL becomes more > >popular and more people start developing software for it, this is more > >likely to occur. > > imo, that is no more or less likely than having two pieces of code > store the same table in the same database. I think what you are > describing would only be a concern if the locks were shared across > databases, however this is not the case. the purpose of advisory > locks is to be 'appplication-defined'. how the application is written > is not part of that concept. we are simply granting the ability to > create a mutex with a number for a name, that is all. Ok, here's a real-world example. RRS (http://rrs.decibel.org) will make use of userlocks if available. RRS by itself isn't very interesting at all; you'd want to use it with something else. Because there's no standard at all for carving up the numbers, I did the best I could by using the OID of one of my functions, because at least back then there was standard OID support. I'm not sure if that even made it into the current version. Using named locks is possibly overkill, but it would be good to at least set aside some chunk of numbers so that it can be done. Likewise I suggested setting aside OIDs above 10k (or whatever a normal database starts numbering at) so that you could at least do per-schema numbering. I'm not asking for a defined solution to how to support multiple different users of locks within the same database. I just want us to set aside (as in, recommend they not be used) some set of numbers so that in the future we could recommend a means of picking lock numbers that will avoid collisions. -- Jim Nasby jim@nasby.net EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote: > I'm not asking for a defined solution to how to support multiple > different users of locks within the same database. I just want us to set > aside (as in, recommend they not be used) some set of numbers so that in > the future we could recommend a means of picking lock numbers that will > avoid collisions. you pretty much already have this, current advisory lock exposes 64 bits of locktag storage. there is 112 bits (3 int4 and 1 int2) available. this is since 8.1 when locktag was reorganized. I was actually going to suggest esposing these fields but had second thoughts due to future proofing issues. note i am not arguing that advisory lock should not be expanded in the future or do string maps, just that at present talking about reserved ranges would just confuse people since the lock space is intentionally generic. merlin
"Merlin Moncure" <mmoncure@gmail.com> writes:
> On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote:
>> This is why I suggested we set aside some range of numbers that should
>> not be used. Doing so would allow adding a better-managed
>> numbering/naming scheme in the future.
> the whole point about advisory locks is that the provided lock space
> is unmanaged.
I think we forgot to document that the lock space is per-database; also,
wouldn't it be a good idea to specifically recommend that advisory locks
be used only in databases that are used just by one application, or a
few cooperating applications? The lack of any permissions checks makes
them fairly unsafe in databases that are used by multiple users.
I don't actually have a problem with the lack of security checks or
key range limitations --- I see advisory locks as comparable to large
objects, which are likewise permissions-free. It's an optional feature
and you just won't use it in databases where permission constraints are
a critical need. The thing that's bothering me is the relative ease of
accidental DoS to applications in *other* databases in the same cluster.
regards, tom lane
On Fri, Sep 22, 2006 at 01:42:48PM -0400, Merlin Moncure wrote: > On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote: > >I'm not asking for a defined solution to how to support multiple > >different users of locks within the same database. I just want us to set > >aside (as in, recommend they not be used) some set of numbers so that in > >the future we could recommend a means of picking lock numbers that will > >avoid collisions. > > you pretty much already have this, current advisory lock exposes 64 > bits of locktag storage. there is 112 bits (3 int4 and 1 int2) > available. this is since 8.1 when locktag was reorganized. I was > actually going to suggest esposing these fields but had second > thoughts due to future proofing issues. > > note i am not arguing that advisory lock should not be expanded in the > future or do string maps, just that at present talking about reserved > ranges would just confuse people since the lock space is intentionally > generic. Ahh, ok, I didn't realize that the total lock space was larger than what's being exposed today. That means we can easily add that stuff in the future and not break anything, which is all I was looking for. -- Jim Nasby jim@nasby.net EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Merlin Moncure" <mmoncure@gmail.com> writes: > > On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote: > >> This is why I suggested we set aside some range of numbers that should > >> not be used. Doing so would allow adding a better-managed > >> numbering/naming scheme in the future. > > > the whole point about advisory locks is that the provided lock space > > is unmanaged. > > I think we forgot to document that the lock space is per-database; also, ok. (ditto user lock legacy) > wouldn't it be a good idea to specifically recommend that advisory locks > be used only in databases that are used just by one application, or a > few cooperating applications? The lack of any permissions checks makes > them fairly unsafe in databases that are used by multiple users. yes and no. so long as it is understood as lock space inside a single database is shared by all sessions? what does it matter how many applications are connecting? applications sharing a single database implied that there is some negotiation of sharing of resources that the server is not aware of. There is no specific objection to n applictions using them on a shared database except for: 1. you must understand that the lock 'namespace' is at database level 2. memory for the lock table is sized for the database cluster, and is shared with standard locks. use it up, and its game over. imo, documentational thrust should be reinforcing those points and not making any specific recommendations which are not derived from them. I dont understand how having one or more applications has anything to do with namespace conflicts, either you have a centrally managed way of managing that namespace or you don't. the idea is just to make sure you have one. I would rather suggest, 'if you have multiple apps connection to the same database, take care to...' etc. going the in the mvcc area which deals (lightly) with locking strategies. overall, the documentation is extremely light on strategies for dealing with concurrency. however, something of a best practices might be in order in light of these considerations. > I don't actually have a problem with the lack of security checks or > key range limitations --- I see advisory locks as comparable to large > objects, which are likewise permissions-free. It's an optional feature > and you just won't use it in databases where permission constraints are > a critical need. The thing that's bothering me is the relative ease of > accidental DoS to applications in *other* databases in the same cluster. you have a point there. merlin
On Sep 22, 2006, at 14:11 , Jim C. Nasby wrote: > On Fri, Sep 22, 2006 at 01:21:57PM -0400, Merlin Moncure wrote: >> On 9/22/06, Jim C. Nasby <jim@nasby.net> wrote: >>> On Fri, Sep 22, 2006 at 12:56:37PM -0400, Merlin Moncure wrote: >>>> the whole point about advisory locks is that the provided lock >>>> space >>>> is unmanaged. for example, in the ISAM system I wrote which hooked >>>> into the acucobol virtual file system interface, I used a global >>>> sequence for row level pessimistic locking but reserved the 48th >>>> bit >>>> for table level locks. This system was extremely effective. on >>>> the >>>> current system I'm working on I use them to lock sequence oid's >>>> plus a >>>> high bit indicator for what i am doing. in short, advisory >>>> locks are >>>> application-defined in concept. >>> >>> Yes, but if you get two pieces of code written by different >>> people using >>> them in the same database, you can get hosed. As PostgreSQL >>> becomes more >>> popular and more people start developing software for it, this is >>> more >>> likely to occur. >> >> imo, that is no more or less likely than having two pieces of code >> store the same table in the same database. I think what you are >> describing would only be a concern if the locks were shared across >> databases, however this is not the case. the purpose of advisory >> locks is to be 'appplication-defined'. how the application is >> written >> is not part of that concept. we are simply granting the ability to >> create a mutex with a number for a name, that is all. > > Except you can put tables (and pretty much all your other objects) > in a > schema, one that's presumably named after your application. That > greatly > removes the odds of conficts. Indeed. In our development environment, we store development, integration, and testing schemas in the same database. This makes it trivial to move testing data to development, for example. If I want to use these locks, it seems I will have to hard-code some offset into each app or hash the schema name and use that as an offset :( In any case, I can't imagine the "wtf?" nightmares an accidental collision would induce. -M
Tom Lane wrote: > "Merlin Moncure" <mmoncure@gmail.com> writes: > > advisory locks still show up as 'userlock' in the pg_locks view. does > > this matter? > > I'm disinclined to change that, because it would probably break existing > client-side code for little gain. I think clarity suggests we should make the heading match the feature, i.e call it "advisory" rather than "userlock". We changed the API, I don't see why keeping the heading makes sense. I think we should leave it unprotected unless we find out that there are unique security problems with advisory locks. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> I'm disinclined to change that, because it would probably break existing
>> client-side code for little gain.
> I think clarity suggests we should make the heading match the feature,
> i.e call it "advisory" rather than "userlock". We changed the API, I
> don't see why keeping the heading makes sense.
(a) we changed a *different* part of the API; I don't see how that
licenses us to whack around anything that's marginally related.
(b) we put up that pgfoundry module so that there would be a backward
compatible solution. Won't be very backward compatible if the locks
look different in pg_locks.
regards, tom lane
"Jim C. Nasby" <jim@nasby.net> writes:
> Ahh, ok, I didn't realize that the total lock space was larger than
> what's being exposed today. That means we can easily add that stuff in
> the future and not break anything, which is all I was looking for.
Yeah --- in particular, we can always add more LOCKTAG values, or make
use of field4 values that are not possible with the current API.
regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> >> An admin who is concerned about this can revoke public access on the
> >> functions for himself ... but should that be the default out-of-the-box
> >> configuration? I feel more comfortable with saying "you have to turn
> >> on this potentially-dangerous feature" than with saying you have to turn
> >> it off.
>
> > I agree with having it turned off by default, at least in 8.2.
>
> Do we have a consensus to do this for 8.2? Or are we going to leave it
> as is? Those are the only two realistic short-term options ...
I'm still of the opinion it'd be better disabled by default, but it
seems that the majority is going the other way. I guess in the end I'd
like to see most of these patched up in such a way that a given user
would be reasonably limited in their ability to DoS the server. That's
not going to happen today though.
Thanks,
Stephen
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> I'm disinclined to change that, because it would probably break existing > >> client-side code for little gain. > > > I think clarity suggests we should make the heading match the feature, > > i.e call it "advisory" rather than "userlock". We changed the API, I > > don't see why keeping the heading makes sense. > > (a) we changed a *different* part of the API; I don't see how that > licenses us to whack around anything that's marginally related. > > (b) we put up that pgfoundry module so that there would be a backward > compatible solution. Won't be very backward compatible if the locks > look different in pg_locks. But is anyone going to know what userlocks is in 1-2 years? We have few people using /contrib/userlocks, but in the future, I bet we have a lot more people using advisory locks, and being confused. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
AgentM <agentm@themactionfaction.com> writes:
> If I want to use these locks, it seems I will have to hard-code some
> offset into each app or hash the schema name and use that as an
> offset :( In any case, I can't imagine the "wtf?" nightmares an
> accidental collision would induce.
That depends entirely on how you are choosing to assign the lock key
numbers. If you use something involving table OID, for example, there
is not a risk of collision from schema considerations.
regards, tom lane
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> (b) we put up that pgfoundry module so that there would be a backward
>> compatible solution. Won't be very backward compatible if the locks
>> look different in pg_locks.
> But is anyone going to know what userlocks is in 1-2 years? We have few
> people using /contrib/userlocks, but in the future, I bet we have a lot
> more people using advisory locks, and being confused.
The reason they're "advisory" is that the current set of functions for
accessing them doesn't enforce anything. That doesn't make the locks
themselves any more or less user-defined than they were before ---
certainly the pg_locks view has got nothing to do with whether they are
advisory or enforced. I do not see a good reason to change it.
It might be worth mentioning in the description of the pg_xxx_lock
functions that the locks they acquire are shown as "userlock" in
pg_locks, but that seems sufficient.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> (b) we put up that pgfoundry module so that there would be a backward > >> compatible solution. Won't be very backward compatible if the locks > >> look different in pg_locks. > > > But is anyone going to know what userlocks is in 1-2 years? We have few > > people using /contrib/userlocks, but in the future, I bet we have a lot > > more people using advisory locks, and being confused. > > The reason they're "advisory" is that the current set of functions for > accessing them doesn't enforce anything. That doesn't make the locks > themselves any more or less user-defined than they were before --- > certainly the pg_locks view has got nothing to do with whether they are > advisory or enforced. I do not see a good reason to change it. > > It might be worth mentioning in the description of the pg_xxx_lock > functions that the locks they acquire are shown as "userlock" in > pg_locks, but that seems sufficient. My point is that if you are going to call them user locks, you then are going to have to call them userlocks in the documentation, which seems pointless, considering that 99% of people who use pg_locks are not applicadtions but users monitoring the system. I just don't see a problem with making it consistent. I don't see the column rename as an API change issue. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On 9/22/06, AgentM <agentm@themactionfaction.com> wrote:
> > Except you can put tables (and pretty much all your other objects)
> > in a
> > schema, one that's presumably named after your application. That
> > greatly
> > removes the odds of conficts.
>
> Indeed. In our development environment, we store development,
> integration, and testing schemas in the same database. This makes it
> trivial to move testing data to development, for example.
>
> If I want to use these locks, it seems I will have to hard-code some
> offset into each app or hash the schema name and use that as an
> offset :( In any case, I can't imagine the "wtf?" nightmares an
> accidental collision would induce.
i think you are obsuring something here. advisory_lock is a mutex
with a numeric name...thats it :) any meaning you impart into that
name is your problem. listen/notify is a similar construct in that
way.
I ran an erp system, one company per schema, using userlock module for
pessimistic row locking with no problems. I used bit shifting to
strip off the high bit (out of 48) for special table locks and other
things. key mechasim was to use a sequence to provide lock id which
was shared by all lockable objects. a domain could be appropriate
here:
create sequence lock_provider;
create domain lockval as bigint default nextval('lock_provider');
and the following becomes standard practice:
create table foo (lv lockval); <--no need for index here
select pg_advisory_lock(lv) from foo where [..];
for bit shifting or special cases you can wrap the lock function, which i did.
merlin
Bruce Momjian <bruce@momjian.us> writes:
> I don't see the column rename as an
> API change issue.
How can you possibly claim it's not an API change?
If you're insistent on this, my recommendation would be to add a new
LOCKTAG value for advisory locks instead of re-using LOCKTAG_USERLOCK.
This would take a little bit more code space but it would preserve the
same pg_locks display for people using the old contrib code, while we
could use "advisory" for locks created by the new code. (Which I still
maintain is a pretty bad way of describing the locks themselves, but
obviously I'm failing to get through to you on that.)
regards, tom lane
On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I don't see the column rename as an > > API change issue. > > How can you possibly claim it's not an API change? > i dunno, i agree with bruce here. we are just changing the output of pg_locks a bit reflecting the change in moving contrib to core. nobody cares about the literal output of pg_locks for userlocks except the old contrib users. compatiblity could be supplied in the pgfoundry module for this as well. i say to leave the lock tables alone and change to 'advsiory'. it just seems odd the way it is. merlin
Merlin Moncure wrote:
> On 9/22/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > I don't see the column rename as an
> > > API change issue.
> >
> > How can you possibly claim it's not an API change?
> >
>
> i dunno, i agree with bruce here. we are just changing the output of
> pg_locks a bit reflecting the change in moving contrib to core.
> nobody cares about the literal output of pg_locks for userlocks except
> the old contrib users. compatiblity could be supplied in the pgfoundry
> module for this as well. i say to leave the lock tables alone and
> change to 'advsiory'. it just seems odd the way it is.
Agreed. I just don't imagine many current user applications referencing
userlocks, and I do imagine confusion in the future by users using the
new API which call them "advisory".
I guess it is a compatibility change, but weighing compatibility against
clarity, I am leaning toward clarity. I assume it is this line that
would be changed:
_("user lock [%u,%u,%u,%u]"),
By my reading of that, that string is language-local, so anyone trying
to parse that directly is going to have a larger problem than our
renaming it for 8.2.
-- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> I guess it is a compatibility change, but weighing compatibility against
> clarity, I am leaning toward clarity. I assume it is this line that
> would be changed:
> _("user lock [%u,%u,%u,%u]"),
You assume wrong ... that has nothing to do with what appears in pg_locks.
Sigh. I'll go break up the locktag into two.
regards, tom lane