Re: get rid of RM_HEAP2_ID - Mailing list pgsql-hackers

On Mon, Oct 06, 2025 at 04:04:00PM +0700, John Naylor wrote:
> Making the XLogRecord header nicer has been proposed several times
> [1][2][3]. In [2], Robert wondered if with 2 bytes of xl_info, we
> could get rid of the separate xl_xact_info. There could be other
> possibilities as well, but I haven't done anything in that direction.
> The attached just gets as far as the modest goal mentioned in the
> subject.

Splitting the generic flags by stealing one of the padding bytes in
XLogRecord sounds like an improvement on clarity grounds, at least.

> 0001: Split xl_info into xl_info for rmgr-specific info and xl_geninfo
> for generic flags. I used the XLogInsertExtended() idea from one of
> Matthias's patches in [3], but wrote the rest a different way to keep
> churn small.

Removing XLR_RMGR_INFO_MASK becomes natural here, causing
XLR_INFO_MASK to also become unnecessary.  One benefit of this
proposal is that we would not need a heap3 once the opcodes are
completely full for the two existing ones, which should be fine enough
for many years to come.  I have noticed after writing this sentence
that it is what Matthias has done in his patch to separate these
fields.  More code churn, but I'd be in favor of fully biting the
bullet and do as Matthias is proposing, getting rid of XLR_INFO_MASK
entirely if we let the full byte at disposal of the RMGRs.

> 0002 and 0003: To simplify the rmgrs that have an opmask and separate
> flag (I saw two obvious ones but I didn't make sure they were the only
> ones), reserve the high 4 bits of xl_info for the "record type" (I see
> xlogstats.c calls it a "recid", so that might be a better name) and
> the lower 4 bits for the flag. Using the same scheme everywhere
> simplifies things.

XLR_REC_TYPE_MASK and XLR_REC_FLAGS_MASK seem a bit confusing to me.
This is an attempt to generalize a rule that two RMGRs have been using
locally.  In short I am not sure what we are gaining here, coming back
to the previous point that we should get rid of XLR_INFO_MASK, IMO.
That's the kind of rules I'd leave up to the RMGRs, so this does not
like a necessary abstraction, at least IMO.

> There are new XLR_* masks whose names reflect their new purpose
> better, but they are the same values as the old ones. XLR_INFO_MASK is
> kept around for compatibility, but it doesn't do anything useful since
> the rmgr has the full byte available. In order for ~XLR_INFO_MASK to
> become a no-op on a full byte, XLR_INFO_MASK has to be zero, which
> looks very odd. Removing it would be clearer, at the cost of more
> churn.

I think that it would be weird to keep XLR_INFO_MASK for compatibility
reasons, with it set at 0, so I would bite the bullet and just get rid
of it entirely.  There was not so long ago a proposal to get entirely
rid of most the footprint of XLR_INFO_MASK in the code, switching to
an equivalent macro:
https://www.postgresql.org/message-id/CAGjhLkNYhNfbhsZhR_sEJ=1VqJmFCzx1BVfWWZ0+it2ucqG-pw@mail.gmail.com

That's also what you are doing here..

> 0004: get rid of RM_HEAP2_ID. This was simple once the prerequisites
> were in place. All of the HEAP2_* macros keep the same name and only
> differ in value. Heap rmgr is completely full so it might be good to
> increase to 5 bits for the record type to give it some breathing room.

This is making an increase in size a prerequisite for anybody who
would want to add a new heap record, which is a problem because this
is pushing this responsibility to the one who would like to add a new
record.  In short, I am not sure that we will ever get rid of heap2,
but I see a pretty good set of arguments to do things so as we'll
never need an equivalent for heap3, or an extra RMGR for any of the
existing RMGRs that use already all of their opcodes (Gin seems to be
using 8 opcodes currently, as one example, so it's full).
--
Michael