On Thu, Oct 13, 2022 at 7:13 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2022-Oct-13, Bharath Rupireddy wrote:
> > Hm. Agree. But, that requires us to include xlogbackup.h in
> > xlog_internal.h for SessionBackupState enum in
> > ResetXLogBackupActivity(). Is that okay?
>
> It's not great, but it's not *that* bad, ISTM, mainly because
> xlog_internal.h will affect less stuff than xlog.h.
This is unfortunately a lot less true than I would like. I count 75
places where we #include "access/xlog.h" and 53 where we #include
"access/xlog_internal.h". And many of those are in frontend code. I
feel like the contents of xlog_internal.h are a bit too eclectic.
Maybe stuff that has to do with the on-disk directory structure, like
XLOGDIR and XLOG_FNAME_LEN, as well as stuff that has to do with where
bytes are located, like XLByteToSeg, should move to another file.
Besides that, which is the biggest part of the file, there's also
stuff that has to do with the page and record format generally (like
XLOG_PAGE_MAGIC and SizeOfXLogShortPHD) and stuff that is used for
certain specific WAL record types (like xl_parameter_change and
xl_restore_point) and some random rmgr-related things (like RmgrData
and the stuff that folllows) and the usual assortment of random GUCs
and global variables (like RecoveryTargetAction and
ArchiveRecoveryRequested). Maybe it doesn't make sense to split this
up into a thousand tiny little header files, but I think some
rethinking would be a good idea, because it really doesn't make much
sense to me to mix stuff that has to do with file-naming conventions,
which a bunch of frontend code needs to know about, together with a
bunch of backend-only things.
--
Robert Haas
EDB: http://www.enterprisedb.com