[PATCH] Refactor: Extract XLogRecord info

Hi,
I refactored the code of extracting XLogRecord info.
In XLogRecord, the high 4 bits in xl_info is used by rmgr.

typedef struct XLogRecord
{
    uint32      xl_tot_len;     /* total len of entire record */
    TransactionId xl_xid;       /* xact id */
    XLogRecPtr  xl_prev;        /* ptr to previous record in log */
    uint8       xl_info;        /* flag bits, see below */
    RmgrId      xl_rmid;        /* resource manager for this record */
    /* 2 bytes of padding here, initialize to zero */
    pg_crc32c   xl_crc;         /* CRC for this record */

    /* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no
padding */

} XLogRecord;

I found lots of the code to get the info as below

XLogRecGetInfo(record) & ~XLR_INFO_MASK

Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
instead of XLR_INFO_MASK(0x0F), which is easier to understand.
Remove XLR_INFO_MASK as it is not used any more.

--
Best regards !
Xiaoran Wang

Hi,

I like the idea of your change as it saves me out of converting-in-my-mind.

And I suggest to create a macro to do this job.
#define getRmgrInfo(info) (info & XLR_RMGR_INFO_MASK)

Then the code can become:
XLogRecGetInfo(record) & ~XLR_INFO_MASK;
-->
getRmgrInfo(XLogRecGetInfo(record));

Thanks,
Steven

在 2025/6/9 14:23, Xiaoran Wang 写道:

Hi,
I refactored the code of extracting XLogRecord info.
In XLogRecord, the high 4 bits in xl_info is used by rmgr.

typedef struct XLogRecord
{
uint32 xl_tot_len; /* total len of entire record */
TransactionId xl_xid; /* xact id */
XLogRecPtr xl_prev; /* ptr to previous record in log */
uint8 xl_info; /* flag bits, see below */
RmgrId xl_rmid; /* resource manager for this record */
/* 2 bytes of padding here, initialize to zero */
pg_crc32c xl_crc; /* CRC for this record */

/* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no
padding */

} XLogRecord;

I found lots of the code to get the info as below

XLogRecGetInfo(record) & ~XLR_INFO_MASK

Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
instead of XLR_INFO_MASK(0x0F), which is easier to understand.
Remove XLR_INFO_MASK as it is not used any more.

--
Best regards !
Xiaoran Wang

Just upload all the patches together.

Xiaoran Wang <fanfuxiaoran@gmail.com> 于2025年6月9日周一 18:25写道：

Steven Niu <niushiji@gmail.com> 于2025年6月9日周一 14:46写道：

Hi,

I like the idea of your change as it saves me out of
converting-in-my-mind.

And I suggest to create a macro to do this job.
#define getRmgrInfo(info) (info & XLR_RMGR_INFO_MASK)

Then the code can become:
XLogRecGetInfo(record) & ~XLR_INFO_MASK;
-->
getRmgrInfo(XLogRecGetInfo(record));

Good idea, found lots of 'XLogRecGetInfo(record) & ~XLR_INFO_MASK;'
in the code.

I add a macro XLogRecRmgrGetInfo(record) in patch 0002.

Thanks,
Steven

在 2025/6/9 14:23, Xiaoran Wang 写道:

Hi,
I refactored the code of extracting XLogRecord info.
In XLogRecord, the high 4 bits in xl_info is used by rmgr.

typedef struct XLogRecord
{
uint32 xl_tot_len; /* total len of entire record */
TransactionId xl_xid; /* xact id */
XLogRecPtr xl_prev; /* ptr to previous record in log */
uint8 xl_info; /* flag bits, see below */
RmgrId xl_rmid; /* resource manager for this record */
/* 2 bytes of padding here, initialize to zero */
pg_crc32c xl_crc; /* CRC for this record */

/* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no
padding */

} XLogRecord;

I found lots of the code to get the info as below

XLogRecGetInfo(record) & ~XLR_INFO_MASK

Actually, we can directly use XLR_RMGR_INFO_MASK(0xF0)
instead of XLR_INFO_MASK(0x0F), which is easier to understand.
Remove XLR_INFO_MASK as it is not used any more.

--
Best regards !
Xiaoran Wang

--
Best regards !
Xiaoran Wang

--
Best regards !
Xiaoran Wang

On Mon, Jun 09, 2025 at 10:54:43PM -0300, Fabrízio de Royes Mello wrote:

The refactoring LGTM but do we really need two patches? IMHO you can just
merge everything into a single patch.

FWIW, I'm not sure what's the benefit of the proposal which comes down
to the removal of a bitwise NOT, except more code conflicts with back
branches.
--
Michael

HI

FWIW, I'm not sure what's the benefit of the proposal which comes down
to the removal of a bitwise NOT, except more code conflicts with back
branches.

Agree

On Tue, Jun 10, 2025 at 3:37 PM Michael Paquier <michael@paquier.xyz>
wrote:

On Mon, Jun 09, 2025 at 10:54:43PM -0300, Fabrízio de Royes Mello wrote:

The refactoring LGTM but do we really need two patches? IMHO you can

just

merge everything into a single patch.

FWIW, I'm not sure what's the benefit of the proposal which comes down
to the removal of a bitwise NOT, except more code conflicts with back
branches.
--
Michael

Steven Niu <niushiji@gmail.com> 于2025年6月10日周二 17:56写道：

I'm confused by the code of XLR_RMGR_INFO_MASK and XLR_INFO_MASK.

According to the definition of masks, the high 4 bits are for rmgr.

/*
* The high 4 bits in xl_info may be used freely by rmgr. The
* XLR_SPECIAL_REL_UPDATE and XLR_CHECK_CONSISTENCY bits can be passed by
* XLogInsert caller. The rest are set internally by XLogInsert.
*/
#define XLR_INFO_MASK 0x0F
#define XLR_RMGR_INFO_MASK 0xF0

However, in function XLogInsert(), there is code:

/*
* The caller can set rmgr bits, XLR_SPECIAL_REL_UPDATE and
* XLR_CHECK_CONSISTENCY; the rest are reserved for use by me.
*/
if ((info & ~(XLR_RMGR_INFO_MASK |
XLR_SPECIAL_REL_UPDATE |
XLR_CHECK_CONSISTENCY)) != 0)
elog(PANIC, "invalid xlog info mask %02X", info);

XLogInsert only allows the rmgr ,XLR_SPECIAL_REL_UPDATE and

XLR_CHECK_CONSISTENCY
set in the info.

#define XLR_SPECIAL_REL_UPDATE 0x01
#define XLR_CHECK_CONSISTENCY 0x02

As the XLR_SPECIAL_REL_UPDATE and XLR_CHECK_CONSISTENCY are of the low 4
bits,
the above code is indicating the low 4 bits are for rmgr too?

No, only the high 4 bits are used for RMGR, see the code under directory
'src/backend/access/rmgrdesc'

'XLR_SPECIAL_REL_UPDATE' and 'XLR_CHECK_CONSISTENCY' are not RMGR info,
but they
can be passed by XLogInsert caller.

Did I misunderstand something?

Thanks，
Steven

--
Best regards !
Xiaoran Wang

Steven Niu <niushiji@gmail.com <mailto:niushiji@gmail.com>> 于2025年6月
10日周二 17:56写道：

I'm confused by the code of XLR_RMGR_INFO_MASK and XLR_INFO_MASK.

According to the definition of masks, the high 4 bits are for rmgr.

/*
* The high 4 bits in xl_info may be used freely by rmgr. The
* XLR_SPECIAL_REL_UPDATE and XLR_CHECK_CONSISTENCY bits can be
passed by
* XLogInsert caller. The rest are set internally by XLogInsert.
*/
#define XLR_INFO_MASK 0x0F
#define XLR_RMGR_INFO_MASK 0xF0

However, in function XLogInsert(), there is code:

/*
* The caller can set rmgr bits, XLR_SPECIAL_REL_UPDATE and
* XLR_CHECK_CONSISTENCY; the rest are reserved for use by me.
*/
if ((info & ~(XLR_RMGR_INFO_MASK |
XLR_SPECIAL_REL_UPDATE |
XLR_CHECK_CONSISTENCY)) != 0)
elog(PANIC, "invalid xlog info mask %02X", info);

XLogInsert only allows the rmgr ,XLR_SPECIAL_REL_UPDATE and
XLR_CHECK_CONSISTENCY
set in the info.

#define XLR_SPECIAL_REL_UPDATE 0x01
#define XLR_CHECK_CONSISTENCY 0x02

As the XLR_SPECIAL_REL_UPDATE and XLR_CHECK_CONSISTENCY are of the
low 4 bits,
the above code is indicating the low 4 bits are for rmgr too?

No, only the high 4 bits are used for RMGR, see the code under directory
'src/backend/access/rmgrdesc'

'XLR_SPECIAL_REL_UPDATE' and 'XLR_CHECK_CONSISTENCY' are not RMGR
info, but they
can be passed by XLogInsert caller.

Did I misunderstand something?

Thanks，
Steven

--
Best regards !
Xiaoran Wang