Re: INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

On Mon, Jan 13, 2014 at 6:45 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> + uint32
> + SpeculativeInsertionIsInProgress(TransactionId xid, RelFileNode rel,
> ItemPointer tid)
> + {

For the purposes of preventing unprincipled deadlocking, commenting
out the following (the only caller of the above) has no immediately
discernible effect with any of the test-cases that I've published:

/* XXX shouldn't we fall through to look at xmax? */
+ /* XXX why? or is that now covered by the above check? */
+ snapshot->speculativeToken =
+ SpeculativeInsertionIsInProgress(HeapTupleHeaderGetRawXmin(tuple),
+ rnode,
+ &htup->t_self);
+
+ snapshot->xmin = HeapTupleHeaderGetRawXmin(tuple);
return true; /* in insertion by other */

I think that the prevention of unprincipled deadlocking is all down to
this immediately prior piece of code, at least in those test cases:

! /*
! * in insertion by other.
! *
! * Before returning true, check for the special case that the
! * tuple was deleted by the same transaction that inserted it.
! * Such a tuple will never be visible to anyone else, whether
! * the transaction commits or aborts.
! */
! if (!(tuple->t_infomask & HEAP_XMAX_INVALID) &&
! !(tuple->t_infomask & HEAP_XMAX_COMMITTED) &&
! !(tuple->t_infomask & HEAP_XMAX_IS_MULTI) &&
! !HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask) &&
! HeapTupleHeaderGetRawXmax(tuple) == HeapTupleHeaderGetRawXmin(tuple))
! {
! return false;
! }

But why should it be acceptable to change the semantics of dirty
snapshots like this, which previously always returned true when
control reached here? It is a departure from their traditional
behavior, not limited to clients of this new promise tuple
infrastructure. Now, it becomes entirely a matter of whether we tried
to insert before or after the deleting xact's deletion (of a tuple it
originally inserted) as to whether or not we block. So in general we
don't get to "keep our old value locks" until xact end when we update
or delete. Even if you don't consider this a bug for existing dirty
snapshot clients (I myself do - we can't rely on deleting a row and
re-inserting the same values now, which could be particularly
undesirable for updates), I have already described how we can take
advantage of deleting tuples while still holding on to their "value
locks" [1] to Andres. I think it'll be very important for multi-master
conflict resolution. I've already described this useful property of
dirty snapshots numerous times on this thread in relation to different
aspects, as it happens. It's essential.

Anyway, I guess you're going to need an infomask bit to fix this, so
you can differentiate between 'promise' tuples and 'proper' tuples.
Those are in short supply. I still think this problem is more or less
down to a modularity violation, and I suspect that this is not the
last problem that will be found along these lines if we continue to
pursue this approach.

[1] http://www.postgresql.org/message-id/CAM3SWZQpLSGPS2Kd=-n6HVYiqkF_mCxmX-Q72ar9UPzQ-X6F6Q@mail.gmail.com
--
Peter Geoghegan