Thread: LockObject patch

LockObject patch

From
Alvaro Herrera
Date:
Hackers,

Here is the LockObject patch I was able to come up with.  It's almost
the same patch that Rod Taylor published two years ago; basically, it
expands LOCKTAG with a ClassId attribute, and provides a LockObject
method to allow locking arbitrary objects.  I omitted the uses of the
new functionality.

I provide this mostly for review purposes; it's not intended to be
applied, because I'll submit it in conjunction with the shared
dependency patch later.

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"El sabio habla porque tiene algo que decir;
el tonto, porque tiene que decir algo" (Platon).

Attachment

Re: LockObject patch

From
Alvaro Herrera
Date:
On Sun, Dec 19, 2004 at 11:40:40PM -0300, Alvaro Herrera wrote:

Hackers,

> Here is the LockObject patch I was able to come up with.  It's almost
> the same patch that Rod Taylor published two years ago; basically, it
> expands LOCKTAG with a ClassId attribute, and provides a LockObject
> method to allow locking arbitrary objects.  I omitted the uses of the
> new functionality.

I just noticed that this uses the current database Id in the LOCKTAG ...
I think I'll just make it use InvalidOid, as we will only use it to lock
shared objects by now.  It can be changed when (and if) we need
different semantics in the future.

--
Alvaro Herrera (<alvherre[@]dcc.uchile.cl>)
Tulio: oh, para qué servirá este boton, Juan Carlos?
Policarpo: No, aléjense, no toquen la consola!
Juan Carlos: Lo apretaré una y otra vez.

Re: LockObject patch

From
Tom Lane
Date:
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> I just noticed that this uses the current database Id in the LOCKTAG ...
> I think I'll just make it use InvalidOid, as we will only use it to lock
> shared objects by now.  It can be changed when (and if) we need
> different semantics in the future.

In that case call it LockSharedObject, or something like that.

            regards, tom lane

Re: LockObject patch

From
Tom Lane
Date:
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> Here is the LockObject patch I was able to come up with.  It's almost
> the same patch that Rod Taylor published two years ago; basically, it
> expands LOCKTAG with a ClassId attribute, and provides a LockObject
> method to allow locking arbitrary objects.

I wonder whether it wouldn't be possible to clean up the "XactLockTable"
kluge with this --- ie, instead of denoting transaction locks by a
special relation ID, denote them by a special class ID.  That might just
move the kluginess from one place to another, but it's worth thinking about.

            regards, tom lane

Re: LockObject patch

From
Alvaro Herrera
Date:
On Mon, Dec 20, 2004 at 03:09:31PM -0500, Tom Lane wrote:
> Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> > Here is the LockObject patch I was able to come up with.  It's almost
> > the same patch that Rod Taylor published two years ago; basically, it
> > expands LOCKTAG with a ClassId attribute, and provides a LockObject
> > method to allow locking arbitrary objects.
>
> I wonder whether it wouldn't be possible to clean up the "XactLockTable"
> kluge with this --- ie, instead of denoting transaction locks by a
> special relation ID, denote them by a special class ID.  That might just
> move the kluginess from one place to another, but it's worth thinking about.

How about locking the special class InvalidOid?  We don't use that ATM
AFAICS.

Anything else would require having a special relation registered, which
is where we are now ...

--
Alvaro Herrera (<alvherre[@]dcc.uchile.cl>)
"La rebeldía es la virtud original del hombre" (Arthur Schopenhauer)

Re: LockObject patch

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> On Mon, Dec 20, 2004 at 03:09:31PM -0500, Tom Lane wrote:
> > Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> > > Here is the LockObject patch I was able to come up with.  It's almost
> > > the same patch that Rod Taylor published two years ago; basically, it
> > > expands LOCKTAG with a ClassId attribute, and provides a LockObject
> > > method to allow locking arbitrary objects.
> >
> > I wonder whether it wouldn't be possible to clean up the "XactLockTable"
> > kluge with this --- ie, instead of denoting transaction locks by a
> > special relation ID, denote them by a special class ID.  That might just
> > move the kluginess from one place to another, but it's worth thinking about.
>
> How about locking the special class InvalidOid?  We don't use that ATM
> AFAICS.

When we roll over OID's do we make sure we never return InvalidOid?

--
  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

Re: LockObject patch

From
Alvaro Herrera
Date:
On Mon, Dec 20, 2004 at 03:52:57PM -0500, Bruce Momjian wrote:

> When we roll over OID's do we make sure we never return InvalidOid?

Yes.  From GetNewObjectId()

    /*
     * Check for wraparound of the OID counter.  We *must* not return 0
     * (InvalidOid); and as long as we have to check that, it seems a good
     * idea to skip over everything below BootstrapObjectIdData too. (This
     * basically just reduces the odds of OID collision right after a wrap
     * occurs.)  Note we are relying on unsigned comparison here.
     */

--
Alvaro Herrera (<alvherre[@]dcc.uchile.cl>)
"Es filósofo el que disfruta con los enigmas" (G. Coli)

Re: LockObject patch

From
Tom Lane
Date:
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> On Mon, Dec 20, 2004 at 03:09:31PM -0500, Tom Lane wrote:
>> I wonder whether it wouldn't be possible to clean up the "XactLockTable"
>> kluge with this --- ie, instead of denoting transaction locks by a
>> special relation ID, denote them by a special class ID.  That might just
>> move the kluginess from one place to another, but it's worth thinking about.

> How about locking the special class InvalidOid?  We don't use that ATM
> AFAICS.

No, that's not an improvement --- if the solution doesn't allow multiple
classes of objects then we've not made any step forward.

We probably ought to step back and think about what objects we need to
be able to lock and what the identifying values are for each kind of
object.  Offhand I think we have:

Whole relations:
    DBID, REL OID        (with special case DBID=0 for shared rels)
Pages within rels:
    DBID, REL OID, BLOCKNUM            (same special case)
Individual tuples (assuming we go the lockmanager route for this):
    DBID, REL OID, BLOCKNUM, OFFNUM        (same special case)
Transactions:
    XID            (would sub-XID be of any use??)
Non-relation objects:
    DBID, CLASS OID, OBJECT OID, OBJECT SUBID
    (it's reasonable to use DBID=0 for shared objects, eg users)
    (we could possibly dispense with subid but for now let's
    assume the same representation as in pg_depend and pg_description)
Userlocks:
    DBID, ID1, ID2
    (current implementation limits ID1 to 16 bits, but we could
    widen that if we wanted)

Basically I think what we've got to decide is how to overlay these
fields and how to tell the six cases apart.

Since subids and offnums are only 16 bits, we could pack all of these
cases into 64 bits with a 16-bit type identifier to distinguish the
cases.  That would mean that LOCKTAG doesn't get any bigger than it is
now, and we'd have plenty of room for expansion still with more types.

            regards, tom lane