Re: advisory locks (was: 8.2 beta blockers)

Started by Merlin Moncureover 19 years ago5 messages
#1Merlin Moncure
mmoncure@gmail.com

On 9/17/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

We have three possible choices for this: do nothing, install a
bug-compatible, allegedly-clean-room implementation in contrib:
http://archives.postgresql.org/pgsql-patches/2006-09/msg00077.php
or put a hopefully-cleaner design into core, eg per my suggestions here:
http://archives.postgresql.org/pgsql-hackers/2006-09/msg00467.php
I favor the third alternative, mainly because by changing the API
we remove all doubt as to whether any "intellectual property"
remains from the original GPL'd code. However, we've got to make up
our minds and get on with it.

two questions: do we need both a shared and unshared variant of
advisory_unlock (im guessing no)? also, are we exposing the mode in
the int4/int4 signature or are all advisory locks assumed to be
exclusive (if yes, which int4 is the lockmode).

merlin

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Merlin Moncure (#1)

"Merlin Moncure" <mmoncure@gmail.com> writes:

two questions: do we need both a shared and unshared variant of
advisory_unlock (im guessing no)?

Yes, because it's possible to hold both shared and exclusive lock
concurrently, so you have to say which you're releasing.

also, are we exposing the mode in
the int4/int4 signature or are all advisory locks assumed to be
exclusive (if yes, which int4 is the lockmode).

Huh?

regards, tom lane

#3Merlin Moncure
mmoncure@gmail.com
In reply to: Merlin Moncure (#1)

On 9/19/06, Merlin Moncure <mmoncure@gmail.com> wrote:

On 9/17/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

We have three possible choices for this: do nothing, install a
bug-compatible, allegedly-clean-room implementation in contrib:
http://archives.postgresql.org/pgsql-patches/2006-09/msg00077.php
or put a hopefully-cleaner design into core, eg per my suggestions here:
http://archives.postgresql.org/pgsql-hackers/2006-09/msg00467.php
I favor the third alternative, mainly because by changing the API
we remove all doubt as to whether any "intellectual property"
remains from the original GPL'd code. However, we've got to make up
our minds and get on with it.

two questions: do we need both a shared and unshared variant of
advisory_unlock (im guessing no)? also, are we exposing the mode in
the int4/int4 signature or are all advisory locks assumed to be
exclusive (if yes, which int4 is the lockmode).

also, is void pg_advisory_lock_shared(int8), etc. not better written as
void pg_advisory_lock_wait(int8). or even better, default
pg_advisory_lock to the 'wait' variant and explicitly declare
pg_advisory_lock_nowait(int8).

there are two things going on here: first, i think we are confusing
the concepts of lockmode and waitmode, and secondly since in most
other places wait locks are default with an optional nowait clause,
how about make advisory locks follow a similar methodology?

rough draft of documentation is done, except for the actual function
definitions :-)

merlin

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Merlin Moncure (#3)

"Merlin Moncure" <mmoncure@gmail.com> writes:

there are two things going on here: first, i think we are confusing
the concepts of lockmode and waitmode, and secondly since in most
other places wait locks are default with an optional nowait clause,
how about make advisory locks follow a similar methodology?

I think *you* are confused about lockmode vs waitmode, but the patch is
not. The functions are

Name | Result data type | Argument data types | Description
pg_advisory_lock | void | bigint | obtain exclusive advisory lock
pg_advisory_lock_shared | void | bigint | obtain shared advisory lock
pg_try_advisory_lock | boolean | bigint | obtain exclusive advisory lock if available
pg_try_advisory_lock_shared | boolean | bigint | obtain shared advisory lock if available
pg_advisory_unlock | boolean | bigint | release exclusive advisory lock
pg_advisory_unlock_shared | boolean | bigint | release shared advisory lock

These also come in flavors taking 2 int4s, which is just a different way
of representing the locked object's identity, it's not different as far
as lock modes go. And there's

pg_advisory_unlock_all | void | | release all advisory locks

which comes in just one flavor.

I don't particularly care about "pg_try_advisory_lock" vs
"pg_advisory_lock_nowait", but it's not entirely obvious which modifier
to put first if we put them both on the end, ie is it
"pg_advisory_lock_shared_nowait" or "pg_advisory_lock_nowait_shared"?
Possibly the names are easier to remember as they are.

regards, tom lane

#5Merlin Moncure
mmoncure@gmail.com
In reply to: Tom Lane (#4)

On 9/19/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Merlin Moncure" <mmoncure@gmail.com> writes:

there are two things going on here: first, i think we are confusing
the concepts of lockmode and waitmode, and secondly since in most
other places wait locks are default with an optional nowait clause,
how about make advisory locks follow a similar methodology?

I think *you* are confused about lockmode vs waitmode, but the patch is
not. The functions are

yep. i realized that after i sent the mail. brain fart...on
reflection lets go with the try variant, its shorter.

merlin