Hi, On 2019-06-24 10:41:06 -0700, Ashwin Agrawal wrote: > Proposing following changes to make predicate locking and checking > functions generic and remove dependency on HeapTuple and Heap AM. We > made these changes to help with Zedstore. I think the changes should > help Zheap and other AMs in general.
Indeed. > - Change PredicateLockTuple() to PredicateLockTID(). So, instead of > passing HeapTuple to it, just pass ItemPointer and tuple insert > transaction id if known. This was also discussed earlier in [1], > don't think was done in that context but would be helpful in future > if such requirement comes up as well. Right. > - CheckForSerializableConflictIn() take blocknum instead of > buffer. Currently, the function anyways does nothing with the buffer > just needs blocknum. Also, helps to decouple dependency on buffer as > not all AMs may have one to one notion between blocknum and single > buffer. Like for zedstore, tuple is stored across individual column > buffers. So, wish to have way to lock not physical buffer but > logical blocknum. Hm. I wonder if we somehow ought to generalize the granularity scheme for predicate locks to not be tuple/page/relation. But even if, that's probably a separate patch. > - CheckForSerializableConflictOut() no more takes HeapTuple nor > buffer, instead just takes xid. Push heap specific parts from > CheckForSerializableConflictOut() into its own function > HeapCheckForSerializableConflictOut() which calls > CheckForSerializableConflictOut(). The alternative option could be > CheckForSerializableConflictOut() take callback function and > callback arguments, which gets called if required after performing > prechecks. Though currently I fell AM having its own wrapper to > perform AM specific task and then calling > CheckForSerializableConflictOut() is fine. I think it's right to move the xid handling out of CheckForSerializableConflictOut(). But I think we also ought to move the subtransaction handling out of the function - e.g. zheap doesn't want/need that. > Attaching patch which makes these changes. Please make sure that there's a CF entry for this (I'm in a plane with a super slow connection, otherwise I'd check). Greetings, Andres Freund
