Re: Serializable lock consistency (was Re: CommitFest wrap-up)

On Dec19, 2010, at 18:06 , Heikki Linnakangas wrote:
> I think this patch is in pretty good shape now.
Apart from the serious deficiency Robert found :-(

I'll still comment on your suggestions though, since
they'd also apply to the solution I suggested on the
other thread.

> The one thing I'm not too happy with is the API for heap_update/delete/lock_tuple. The return value is:
>
> <snipped comment citation>
>
> That's quite complicated. I think we should bite the bullet and add a couple of more return codes to explicitly tell
thecaller what happened. I propose: 

Yeah, it's a bit of a mess. On the other hand, heap_{update,delete,lock_tuple} are only called from very few places
(simple_heap_update,simple_heap_delete, ExecUpdate, ExecLockRows and GetTupleForTrigger). Of these, only ExecUpdate and
ExecLockRowscare for update_xmax and ctid. 

> HeapTupleMayBeUpdated- the tuple was actually updated (same as before)
> HeapTupleSelfUpdated - the tuple was updated by a later command in same xact (same as before)
> HeapTupleBeingUpdated - concurrent update in progress (same as before)
> HeapTupleUpdated - the tuple was updated by another xact. *update_xmax and *ctid are set to point to the replacement
tuple.
> HeapTupleDeleted - the tuple was deleted by another xact
> HeapTupleLocked - lockcheck_snapshot was given, and the tuple was locked by another xact

Hm, I'm not happy with HeapTupleMayBeUpdated meaning "The tuple was updated" while HeapTupleUpdated means "The tuple
wasn'tupdated, a concurrent transaction beat us to it" seems less than ideal. On the whole, I'd much rather have a
secondenum, say HO_Result for heap operation result, instead of miss-using HTSU_Result for this. HO_Result would have
thefollowing possible values 

HeapOperationCompleted - the tuple was updated/deleted/locked
HeapOperationSelfModified - the tuple was modified by a later command in the same xact. We don't distinguish the
differentcases here since none of the callers care. 
HeapOperationBeingModified - the tuple was updated/deleted/locked (and the lock conflicts) by a transaction still
in-progress.
HeapOperationConcurrentUpdate - the tuple was updated concurrently. *update_xmax and *ctid are set to point to the
replacementtuple. 
HeapOperationConcurrentDelete - the tuple was deleted concurrently.
HeapOperationConcurrentLock - the tuple was locked concurrently (only if lockcheck_snapshot was provided).

If we do want to keep heap_{update,delete,lock_tuple} result HTSU_Result, we could also add an output parameter of type
HTSU_Failurewith the possible values 

HTSUConcurrentUpdate
HTSUConcurrentDelete
HTSUConcurrentLock

and set it accordingly if we return HeapTupleUpdated.

> I'm not sure how to incorporate that into the current heap_delete/update/lock_tuple functions and
HeapTupleSatisfiesUpdate.It would be nice to not copy-paste the logic to handle those into all three functions. Perhaps
thatcommon logic starting with the HeapTupleSatisfiesUpdate() call could be pulled into a common function. 


Hm, the logic in heap_lock_tuple is quite different from heap_delete and heap_update, since it needs to deal with
share-modelock acquisition. But for heap_{update,delete} unifying the logic should be possible. 

best regards,
Florian Pflug