Thank you for the comments. This will help move the discussion
forward.
At Fri, 5 Jan 2024 15:17:11 -0500, Robert Haas <robertmhaas@gmail.com> wrote in
> On Thu, Mar 23, 2023 at 11:02 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> > [ new patch ]
>
> Well, I guess nobody is too excited about fixing this, because it's
> been another 10 months with no discussion. Andres doesn't even seem to
> think this is as much a bug as it is a limitation, for all that it's
> filed in the CF application under bug fixes. I kind of wonder if we
> should just close this entry in the CF, but I'll hold off on that for
> now.
Perhaps you are correct. Ultimately, this issue is largely
theoretical, and I don't believe anyone would be inconvenienced by
imposing this contraint.
> * note: all ids in xip[] satisfy xmin <= xip[i] < xmax
> */
>
> I have to say that I don't like this at all. It's bad enough that we
> already use the xip/subxip arrays in two different ways depending on
> the situation. Increasing that to three different ways seems painful.
> How is anyone supposed to keep track of how the array is being used at
> which point in the code?
I understand. So, summirizing the current state briefly, I believe it
as follows:
a. snapbuild lacks a means to differentiate between top and sub xids
during snapshot building.
b. Abusing takenDuringRecovery could lead to potential issues, so it
has been rejected.
c. Dynamic sizing of xip is likely to have a significant impact on
performance (as mentioned in the comments of GetSnapshotData).
d. (new!) Adding a third storing method is not favored.
As a method to satisfy these prerequisites, I think one direction
could be to split takenDuringRecovery into flags indicating the
storage method and creation timing. I present this as a last-ditch
effort. It's a rough proposal, and further validation should be
necessary. If this direction is also not acceptable, I'll proceed with
removing the CF entry.
> If we are going to do that, I suspect it needs comment updates in more
> places than what the patch does currently. For instance:
>
> + if (newxcnt < xiplen)
> + newxip[newxcnt++] = xid;
> + else
> + newsubxip[newsubxcnt++] = xid;
>
> Just imagine coming across this code in 5 or 10 years and finding that
> it had no comment explaining anything. Yikes!
^^;
> Aside from the details of the patch, and perhaps more seriously, I'm
> not really clear that we have consensus on an approach. A few
> different proposals seem to have been floated, and it doesn't seem
> like anybody hates anybody else's idea completely, but it doesn't
> really seem like everyone agrees on what to do, either.
I don't fully agree with that.It's not so much that I dislike other
proposals, but rather that we haven't been able to find a definitive
solution that stands out.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center