On Thu, Oct 13, 2022 at 7:13 AM Alvaro Herrera <alvhe...@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
