Re: INSERT...ON DUPLICATE KEY LOCK FOR UPDATE - Mailing list pgsql-hackers

On 12/31/2013 09:18 AM, Peter Geoghegan wrote:
> On Sun, Dec 29, 2013 at 9:09 AM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>>>> While mulling this over further, I had an idea about this: suppose we
>>>> marked the tuple in some fashion that indicates that it's a promise
>>>> tuple.  I imagine an infomask bit, although the concept makes me wince
>>>> a bit since we don't exactly have bit space coming out of our ears
>>>> there.  Leaving that aside for the moment, whenever somebody looks at
>>>> the tuple with a mind to calling XactLockTableWait(), they can see
>>>> that it's a promise tuple and decide to wait on some other heavyweight
>>>> lock instead.  The simplest thing might be for us to acquire a
>>>> heavyweight lock on the promise tuple before making index entries for
>>>> it, and then have callers wait on that instead always instead of
>>>> transitioning from the tuple lock to the xact lock.
>>
>> Yeah, that seems like it should work. You might not even need an infomask
>> bit for that; just take the "other heavyweight lock" always before calling
>> XactLockTableWait(), whether it's a promise tuple or not. If it's not,
>> acquiring the extra lock is a waste of time but if you're going to sleep
>> anyway, the overhead of one extra lock acquisition hardly matters.
>
> Are you suggesting that I lock the tuple only (say, through a special
> LockPromiseTuple() call), or lock the tuple *and* call
> XactLockTableWait() afterwards? You and Robert don't seem to be in
> agreement about which here.

I meant the latter, ie. grab the new kind of lock first, then check if 
the tuple is still there, and then call XactLockTableWait() as usual.

>> The inserter has to acquire the heavyweight lock before releasing the buffer
>> lock, because otherwise another inserter (or deleter or updater) might see
>> the tuple, acquire the heavyweight lock, and fall to sleep on
>> XactLockTableWait(), before the inserter has grabbed the heavyweight lock.
>> If that race condition happens, you have the original problem again, ie. the
>> updater unnecessarily waits for the inserting transaction to finish, even
>> though it already killed the tuple it inserted.
>
> Right. Can you suggest a workaround to the above problems?

Umm, I did, in the next paragraph ;-) :

> That seems easy to avoid. If the heavyweight lock uses the
> transaction id as the key, just like
> XactLockTableInsert/XactLockTableWait, you can acquire it before
> doing the insertion.

Let me elaborate that. The idea is to have new heavy-weight lock that's 
just like the transaction lock used by 
XactLockTableInsert/XactLockTableWait, but separate from that. Let's 
call it PromiseTupleInsertionLock. The insertion procedure in INSERT ... 
ON DUPLICATE looks like this:

1. PromiseTupleInsertionLockAcquire(<my xid>)
2. Insert heap tuple
3. Insert index tuples
4. Check if conflict happened. Kill the already-inserted tuple on conflict.
5. PromiseTupleInsertionLockRelease(<my xid>)

IOW, the only change to the current patch is that you acquire the new 
kind of lock before starting the insertion, and you release it after 
you've killed the tuple, or you know you're not going to kill it.

- Heikki