Horton

Thread

  1. [PATCH] Refactor: Extract XLogRecord info

    Xiaoran Wang <fanfuxiaoran@gmail.com> — 2025-06-09T06:23:17Z

    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

  2. Re: [PATCH] Refactor: Extract XLogRecord info

    Steven Niu <niushiji@gmail.com> — 2025-06-09T06:45:50Z

    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

  3. Re: [PATCH] Refactor: Extract XLogRecord info

    wenhui qiu <qiuwenhuifx@gmail.com> — 2025-06-09T07:17:19Z

    HI
> 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))
+1 Agreed, this makes the code more readable.

Thanks

On Mon, Jun 9, 2025 at 2:46 PM Steven Niu <niushiji@gmail.com> wrote:

> 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
>
>
>
>

  4. Re: [PATCH] Refactor: Extract XLogRecord info

    Xiaoran Wang <fanfuxiaoran@gmail.com> — 2025-06-09T10:25:54Z

    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

  5. Re: [PATCH] Refactor: Extract XLogRecord info

    Xiaoran Wang <fanfuxiaoran@gmail.com> — 2025-06-09T10:30:47Z

    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

  6. Re: [PATCH] Refactor: Extract XLogRecord info

    Steven Niu <niushiji@gmail.com> — 2025-06-10T01:39:37Z

    LGTM. I have no more comments.

Regards,
Steven

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

> 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
>

  7. Re: [PATCH] Refactor: Extract XLogRecord info

    Fabrízio de Royes Mello <fabriziomello@gmail.com> — 2025-06-10T01:54:43Z

    On Mon, 9 Jun 2025 at 22:40 Steven Niu <niushiji@gmail.com> wrote:

> LGTM. I have no more comments.
>
>
The refactoring LGTM but do we really need two patches? IMHO you can just
merge everything into a single patch.

Fabrízio de Royes Mello

  8. Re: [PATCH] Refactor: Extract XLogRecord info

    Michael Paquier <michael@paquier.xyz> — 2025-06-10T07:37:23Z

    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

  9. Re: [PATCH] Refactor: Extract XLogRecord info

    wenhui qiu <qiuwenhuifx@gmail.com> — 2025-06-10T08:00:29Z

    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
>

  10. Re: [PATCH] Refactor: Extract XLogRecord info

    Steven Niu <niushiji@gmail.com> — 2025-06-10T09:56:09Z

    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);

#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?

Did I misunderstand something?

Thanks，
Steven

wenhui qiu <qiuwenhuifx@gmail.com> 于2025年6月10日周二 16:00写道：

> 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
>>
>

  11. Re: [PATCH] Refactor: Extract XLogRecord info

    Xiaoran Wang <fanfuxiaoran@gmail.com> — 2025-06-11T02:13:41Z

    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

  12. Re: [PATCH] Refactor: Extract XLogRecord info

    Steven Niu <niushiji@gmail.com> — 2025-06-11T02:27:06Z

     Hi, Xiaoran,

I see. The code is checking if the bits other than rmgr bits,
XLR_SPECIAL_REL_UPDATE and XLR_CHECK_CONSISTENCY are used.

Thanks for the explanation.

Steven

Xiaoran Wang <fanfuxiaoran@gmail.com> 于2025年6月11日周三 10:13写道：

>
>
> 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
>

  13. Re: [PATCH] Refactor: Extract XLogRecord info

    Steven Niu <niushiji@gmail.com> — 2025-06-11T02:27:43Z

    Hi, Xiaoran,

I see. The code is checking if the bits other than rmgr bits,
XLR_SPECIAL_REL_UPDATE and XLR_CHECK_CONSISTENCY are used.

Thanks for explanation.

Steven

在 2025/6/11 10:13, 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