Thanks for looking this!
At Thu, 23 Mar 2023 14:15:12 +0530, Dilip Kumar <dilipbalaut@gmail.com> wrote in
> On Thu, Mar 23, 2023 at 10:53 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> Thanks for working on this, your idea looks fine but my only worry is
> that in the future if someone tries to change the logic of
> XidInMVCCSnapshot() then they must be aware that the snap->xip array
> and snap->subxip array no long distinguishes the top xids and subxids.
Yeah, I had the same thought when I was working on the posted version.
> I agree with the current logic if we are not marking sub-overflow then
> there is no issue, so can we document this in the SnapshotData
> structure?
(I found that it was alrady mentioned...)
In a unpublished version (what I referred to as "a mess"), I added a
flag called "topsub_mixed" to SnapshotData, indicating that XIDs of
top and sub transactions are stored in xip and subxip arrays in a
mixed manner. However, I eventually removed it since it could only be
used for sanity checks related to suboverflowed.
I inserted the following sentense in the middle of the comments for
xip and subxip.
> In the case of !suboverflowed, there's a situation where this
> contains both top and sub-transaction IDs in a mixed manner.
And added similar a similar sentense to a comment of
XidInMVCCSnapshot.
While doning this, I realized that we could simplify and optimize XID
search code by combining the two XID arrays. If !suboverflowed, the
array stored all active XIDs of both top and
sub-transactions. Otherwise it only stores active top XIDs and maybe
XIDs of some sub-transactions. If many subXIDs are stored when
overflowed, there might lead to some degradation but I think the win
we gain from searching just one XID array in most cases outweighs
that. (I didn't do this (of course) in this version.)
> Also, there are some typos in the patch
> /idetify/identify
> /carete/create
> /Aallocate/Allocate
Oops! Thanks for pointing out them.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
