On Tue, Oct 14, 2025 at 03:20:16PM +0300, Heikki Linnakangas wrote:
> I'm not sure I agree with the premise that we should try to get rid of
> RM_HEAP2_ID. There's nothing wrong with that scheme as such. As an
> alternative, we could easily teach e.g pg_waldump to treat RM_HEAP_ID and
> RM_HEAP2_ID the same for statistics purposes.
Yeah, I'd rather keep heap2 as well. As long as there is more room
for the record IDs, we're still going to need it in the long run.
> This patch consumes one of the padding bytes. That's not entirely free, as
> there is an opportunity cost: we could squeeze out the padding bytes and
> save 2 bytes on every WAL record instead.
Do you recall an alternative where it would have been possible to save
2 bytes for each record by removing the padding, and still have the
full byte of xl_info be usable freely by each RMGR? I cannot recall
any magic based on how XLogRecord is designed now, but perhaps I
have missed an argument.
We could move out xl_xid, which should not be required for all
records, shaving 4 bytes from the base XLogRecord. I'm afraid of the
duplication this would create if we push this data to each RMGR, which
would, I guess, require a new RMGR callback to retrieve this field on
a per-record basis. But perhaps it would not be that bad.
> In summary:
>
> - Rename 'xl_info' to 'xl_rminfo' to make it more clear that it's
> RMGR-specific.
>
> - Rename 'xl_geninfo' to 'xl_flags'. I guess the 'gen' meant 'generic' or
> 'general' to distinguish from the rmgr-specific field. But we don't use a
> 'gen' prefix like that for any of the other non-rmgr-specific fields. We
> could keep it under the old 'xl_info' name instead, but it's nice to rename
> it to avoid confusion with the old xl_info field. It now only contains
> flags, so 'xl_flags' seems appropriate.
>
> - Reorder the fields so that 'xl_rmid' comes before 'xl_rminfo'. I find that
> more intuitive, because the contents of 'xl_rminfo' depends on 'xl_rmid'.
Okay.
> While we're at it, I wonder if it would be more clear to have explicit
> 'xl_padding' field for the unused byte.
Adding an extra field to document the padding sounds like a good idea
here. It should always be zero, so perhaps it could even be used as a
sanity check?
--
Michael
