Re: FOR KEY LOCK foreign keys

Excerpts from Noah Misch's message of vie feb 11 04:13:22 -0300 2011:

Hello,

First, thanks for the very thorough review.

> On Thu, Jan 13, 2011 at 06:58:09PM -0300, Alvaro Herrera wrote:

> Incidentally, HeapTupleSatisfiesMVCC has some bits of code like this (not new):
>
> /* MultiXacts are currently only allowed to lock tuples */
> Assert(tuple->t_infomask & HEAP_IS_LOCKED);
>
> They're specifically only allowed for SHARE and KEY locks, right?
> heap_lock_tuple seems to assume as much.

Yeah, since FOR UPDATE acquires an exclusive lock on the tuple, you
can't have a multixact there. Maybe we can make the assert more
specific; I'll have a look.

> [ test case with funny visibility behavior ]

Looking into the visibility bug.

> > I published about this here:
> > http://commandprompt.com/blogs/alvaro_herrera/2010/11/fixing_foreign_key_deadlocks_part_2/
> >
> > So, as a rough design,
> >
> > 1. Create a new SELECT locking clause. For now, we're calling it SELECT FOR KEY LOCK
> > 2. This will acquire a new type of lock in the tuple, dubbed a "keylock".
> > 3. This lock will conflict with DELETE, SELECT FOR UPDATE, and SELECT FOR SHARE.
>
> It does not conflict with SELECT FOR SHARE, does it?

It doesn't; I think I copied old text there. (I had originally thought
that they would conflict, but I had to change that due to implementation
restrictions).

> The odd thing here is the checking of an outside condition to decide whether
> locks conflict. Normally, to get a different conflict list, we add another lock
> type. What about this?
>
> FOR KEY SHARE conflicts with FOR KEY UPDATE
> FOR SHARE conflicts with FOR KEY UPDATE, FOR UPDATE
> FOR UPDATE conflicts with FOR KEY UPDATE, FOR UPDATE, FOR SHARE
> FOR KEY UPDATE conflicts with FOR KEY UPDATE, FOR UPDATE, FOR SHARE, FOR KEY SHARE

Hmm, let me see about this.

> > 3. The original tuple needs to be marked with the Cmax of the locking
> > command, to prevent it from being seen in the same transaction.
>
> Could you elaborate on this requirement?

Consider an open cursor with a snapshot prior to the lock. If we leave
the old tuple as is, the cursor would see that old tuple as visible.
But the locked copy of the tuple is also visible, because the Cmax is
just a locker, not an updater.

> > 4. A non-conflicting update to the tuple must carry forward some fields
> > from the original tuple into the updated copy. Those include Xmax,
> > XMAX_IS_MULTI, XMAX_KEY_LOCK, and the CommandId and COMBO_CID flag.
>
> HeapTupleHeaderGetCmax() has this assertion:
>
> /* We do not store cmax when locking a tuple */
> Assert(!(tup->t_infomask & (HEAP_MOVED | HEAP_IS_LOCKED)));
>
> Assuming that assertion is still valid, there will never be a HEAP_COMBOCID flag
> to copy. Right?

Hmm, I think the assert is wrong, but I'm still paging in the details of
the patch after being away from it for so long. Let me think more about it.

> [ Lots more stuff ]

I'll give careful consideration to all this.

Thanks again for the detailed review.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support