Hi On January 29, 2019 8:04:55 AM PST, Petr Jelinek <petr.jeli...@2ndquadrant.com> wrote: >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.
Not sure why that's relevant - what you'd do is to attempt the update, if it succeeds: great. If not, then you'd do the lock tuple and after that do the conflict resolution. Less WAL and cpu for the common case, same WAL as now for the conflict, with just a small bit of additional CPU costs for the latter. Either way, right now it's definitely superfluous... Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
