Hi, On 2019-08-05 20:58:05 +1200, Thomas Munro wrote: > On Sat, Aug 3, 2019 at 11:56 AM Ashwin Agrawal <aagra...@pivotal.io> wrote: > > On Wed, Jul 31, 2019 at 2:06 PM Andres Freund <and...@anarazel.de> wrote: > >> I'm also a bit confused why we don't need to pass in the offset of the > >> current tuple, rather than the HOT root tuple here. That's not related > >> to this patch. But aren't we locking the wrong tuple here, in case of > >> HOT? > > > > Yes, root is being locked here instead of the HOT. But I don't have full > > context on the same. If we wish to fix it though, can be easily done now > > with the patch by passing "tid" instead of &(heapTuple->t_self). > > Here are three relevant commits:
Thanks for digging! > 1. Commit dafaa3efb75 "Implement genuine serializable isolation > level." (2011) locked the root tuple, because it set t_self to *tid. > Possibly due to confusion about the effect of the preceding line > ItemPointerSetOffsetNumber(tid, offnum). > > 2. Commit commit 81fbbfe3352 "Fix bugs in SSI tuple locking." (2013) > fixed that by adding ItemPointerSetOffsetNumber(&heapTuple->t_self, > offnum). Hm. It's not at all sure that it's ok to report the non-root tuple tid here. I.e. I'm fairly sure there was a reason to not just set it to the actual tid. I think I might have written that up on the list at some point. Let me dig in memory and list. Obviously possible that that was also obsoleted by parallel changes. > 3. Commit b89e151054a "Introduce logical decoding." (2014) also did > ItemPointerSet(&(heapTuple->t_self), BufferGetBlockNumber(buffer), > offnum), for the benefit of historical MVCC snapshots (unnecessarily, > considering the change in the commit #2), but then, intending to > "reset to original, non-redirected, tid", clobbered it, reintroducing > the bug fixed by #2. > My first guess is that commit #3 above was developed before commit #2, > and finished up clobbering it. Yea, that sounds likely. > This must be in want of an isolation test, but I haven't yet tried to > get my head around how to write a test that would show the difference. Indeed. Greetings, Andres Freund
