From: Andres Freund <andres@2ndquadrant.com>
To: pgsql-hackers@postgresql.org
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Subject: Re: foreign key locks
Date: Sat, 27 Oct 2012 00:06:53 +0200
Message-ID: <201210270006.54728.andres@2ndquadrant.com> (raw)
In-Reply-To: <20121018195820.GO3763@alvh.no-ip.org>
References: <4FEAB90A0200002500048B7D@gw.wicourts.gov>
<1346389151-sup-8379@alvh.no-ip.org>
<20121018195820.GO3763@alvh.no-ip.org>
On Thursday, October 18, 2012 09:58:20 PM Alvaro Herrera wrote:
> Here is version 22 of this patch. This version contains fixes to issues
> reported by Andres, as well as a rebase to latest master.
Ok, I now that pgconf.eu has ended I am starting to do a real review:
* Is it ok to make FOR UPDATE somewhat weaker than before? In 9.2 and earlier
you could be sure that if you FOR UPDATE'ed a row you could delete it. Unless
I miss something now this will not block somebody else acquiring a FOR KEY
SHARE lock, so this guarantee is gone.
This seems likely to introduce subtle problems in user applications.
I propose renaming FOR UPDATE => FOR NO KEY UPDATE, FOR KEY UPDATE => FOR
UPDATE or similar (or PREVENT KEY UPDATE?). That keeps the old meaning of FOR
UPDATE.
You write "SELECT FOR UPDATE is a standards-compliant exclusive lock". I
didn't really find all that much about the semantics of FOR UPDATE on cursors
in the standard other than "The operations of update and delete are permitted
for updatable cursors, subject to constraining Access Rules.".
* I would welcome adding some explanatory comments about the point of
TupleLockExtraInfo and MultiXactStatusLock at the respective definition.
* Why do we have the HEAP_XMAX_IS_MULTI && HEAP_XMAX_LOCK_ONLY case?
* I think some of the longer comments could use the attention of a native
speaker, unfortunately thats not me.
* I am still uncomfortable with the FOR SHARE deoptimization. I have seen
people lock larger parts of their table to make some READ COMMITTED things
actually safe.
Is there any problem retaining the non XMAX_IS_MULTI behaviour except space in
infomask? That seems solveable by something like
#define HEAP_XMAX_SHR_LOCK 0x0010
#define HEAP_XMAX_EXCL_LOCK 0x0040
#define HEAP_XMAX_KEYSHR_LOCK (HEAP_XMAX_SHR_LOCK | HEAP_XMAX_EXCL_LOCK)
and somewhat more complex expressions when testing the locks ((infomask &
HEAP_XMAX_KEYSHR_LOCK ) == HEAP_XMAX_KEYSHR_LOCK, etc).
* In heap_lock_tuple's XMAX_IS_MULTI case
for (i = 0; i < nmembers; i++)
{
if (TransactionIdIsCurrentTransactionId(members[i].xid))
{
LockTupleMode membermode;
membermode =
TUPLOCK_from_mxstatus(members[i].status);
if (membermode > mode)
{
if (have_tuple_lock)
UnlockTupleTuplock(relation, tid, mode);
pfree(members);
return HeapTupleMayBeUpdated;
}
}
}
why is it membermode > mode and not membermode >= mode?
* Is the case of a a non-key-update followed by a key-update actually handled
when doing a heap_lock_tuple with mode = LockTupleKeyShare and follow_updates
= false? I don't really see how, so it seems to deserve at least a comment.
But then I don't yet understand why follow_update makes sense.
* In heap_lock_tuple with mode == LockTupleUpdate && infomask &
HEAP_XMAX_IS_MULTI, were leaking members when doing goto l3. Probably not
relevant, but given the code tries to be careful everywhere else...
We might also leak in the members == 0 case there, not sure yet.
Ok, this is at about 35% of the diff in my second pass, but I just arrived back
in Berlin, and this seems useful enough on its own...
Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: andres@2ndquadrant.com, alvherre@2ndquadrant.com
Subject: Re: foreign key locks
In-Reply-To: <201210270006.54728.andres@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox