On 29/01/2019 16:28, Andres Freund wrote: > Hi, > > On January 29, 2019 4:19:52 AM PST, Petr Jelinek > <petr.jeli...@2ndquadrant.com> wrote: >> Hi, >> >> On 20/01/2019 21:03, Andres Freund wrote: >>> Hi, >>> >>> Currently RelationFindReplTupleByIndex(), RelationFindReplTupleSeq() >>> lock the found tuple. I don't quite get what that achieves - why >> isn't >>> dealing with concurrency in the table_update/delete calls at the >>> callsites sufficient? As far as I can tell there's no meaningful >>> concurrency handling in the heap_lock_tuple() paths, so it's not like >> we >>> follow update chains or anything. >>> >> >> Yeah that's leftover from the conflict detection/handling code that I >> stripped away to keep the patched manageable size-wise. As things stand >> now we could remove that and use normal heap_update instead of simple >> variant. It'll be likely be needed again if we add conflict handling in >> the future, but perhaps we could be smarter about it then (i.e. I can >> imagine that it will be per table anyway, not necessarily default >> behavior). > > Why does conflict handling need the unconditional lock? Wouldn't just doing > that after an initial heap_update returned HeapTupleUpdated make more sense? > And wouldn't it need to reckeck the row afterwards as well? >
To prevent tuple changing from under the conflict resolution logic - we need to make sure that whatever tuple the conflict resolution logic gets is the current one. If the tuple is locked you won't get the HeapTupleUpdated in heap_update anymore (which is why we can use simple_heap_update). In any case we don't have conflict resolution at the moment so it's probably moot right now. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
