On Fri, Mar 25, 2022 at 12:18 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2022-03-24 15:02:29 +0530, Bharath Rupireddy wrote:
> > On Thu, Mar 24, 2022 at 10:22 AM Kyotaro Horiguchi
> > > This doesn't seem to be a part of xlogreader. Couldn't we add a new
> > > module "xlogstats"? XLogRecGetBlockRefInfo also doesn't seem to me as
> > > a part of xlogreader, the xlogstats looks like a better place.
> >
> > I'm not sure if it's worth adding new files xlogstats.h/.c just for 2
> > structures, 1 macro, and 2 functions with no plan to add new stats
> > structures or functions. Since xlogreader is the one that reads the
> > WAL, and is being included by both backend and other modules (tools
> > and extensions) IMO it's the right place. However, I can specify in
> > xlogreader that if at all the new stats related structures or
> > functions are going to be added, it's good to move them into a new
> > header and .c file.
>
> I don't like that location for XLogRecGetBlockRefInfo(). How about putting it
> in xlogdesc.c - that kind of fits?
Done.
> And what do you think about creating src/backend/access/rmgrdesc/stats.c for
> XLogRecStoreStats()? It's not a perfect location, but not too bad either.
>
> XLogRecGetLen() would be ok in xlogreader, but stats.c also would work?
I've added a new xlogstats.c/.h (as suggested by Kyotaro-san as well)
file under src/backend/access/transam/. I don't think the new file
fits well under rmgrdesc.
Attaching v15 patch-set, please have a look at it.
Regards,
Bharath Rupireddy.