At Mon, 25 Oct 2021 10:34:27 +0530, Amul Sul <sulamul@gmail.com> wrote in
> On Mon, Oct 25, 2021 at 7:02 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > At Fri, 22 Oct 2021 18:43:52 +0530, Amul Sul <sulamul@gmail.com> wrote in
> > > Any thoughts about the patch posted previously?
> >
> > Honestly, xlogreader looks fine with the current shape. The reason is
> > that it seems cleaner as an interface boundary since the caller of
> > xlogreader doesn't need to know about the details of xlogreader. The
> > current code nicely hides the end+1 confusion.
> >
> > Even if we want to get rid of global variables in xlog.c, I don't
> > understand why we remove only abortedRecPtr. That change makes things
> > more complex as a whole by letting xlog.c be more conscious of
> > xlogreader's internals. I'm not sure I like that aspect of the patch.
> >
>
> Because we have other ways to get abortedRecPtr without having a
> global variable, but we don't have such a way for missingContrecPtr,
> AFAICU.
That depends on the reason why you want to get rid of the glboal
variables. Since we restart WAL reading before reading the two
variables so we can not rely on the xlogreader's corresponding
members. So we need another set of variables to preserve the values
beyond the restart.
> I agree using global variables makes things a bit easier, but those
> are inefficient when you want to share those with other processes --
> that would add extra burden to shared memory.
We could simply add a new member in XLogCtlData. Or we can create
another struct for ReadRecord's (not XLogReader's) state then allocate
shared memory to it. I don't think it is the right solution to infer
it from another variable using knowledge of xlogreader's internals.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
