Re: Return value of XLogInsertRecord() for XLOG_SWITCH record

From: 반지현 <rring0727(at)gmail(dot)com>
To: pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: cca5507 <cca5507(at)qq(dot)com>
Subject: Re: Return value of XLogInsertRecord() for XLOG_SWITCH record
Date: 2026-04-21 04:25:09
Message-ID: CA+6Fkk2kbhFtxi=zYGjOP7ampqS4FPCd12cqnVcSad0HkbmtHw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi ChangAo,

I tested v1 of this patch.

Environment:
- PostgreSQL master (commit 9b43e6793b0)
- Ubuntu 24.04, gcc 13.3.0, x86_64
- Configured with --enable-cassert --enable-debug --enable-tap-tests

The patch applies cleanly and builds without new warnings.

Testing performed:
- make check: PASS
- make check-world: PASS
- make -C src/test/recovery check: PASS (599 tests)
In particular, t/043_no_contrecord_switch.pl and t/039_end_of_wal.pl
both pass, which exercise the XLOG_SWITCH boundary handling.

Manual verification:
I ran pg_switch_wal() twice with some activity in between and observed
that the returned LSN and subsequent pg_current_wal_lsn() values are
consistent with segment boundaries and page headers:

SELECT pg_switch_wal(); -- 0/0178FE38
SELECT pg_current_wal_lsn(); -- 0/02000000 (new segment)

-- ... some DDL and INSERT ...

SELECT pg_switch_wal(); -- 0/02026528
SELECT pg_current_wal_lsn(); -- 0/03000060 (new segment + 0x60
-- == SizeOfXLogLongPHD)

The 0x60 offset matches the long page header size, which is what the
original code was computing via the explicit SizeOfXLogLongPHD /
SizeOfXLogShortPHD branches. The refactored version using
XLogBytePosToEndRecPtr(XLogRecPtrToBytePos(StartPos) +
MAXALIGN(SizeOfXLogRecord)) produces the same result while matching
the pattern used elsewhere in XLogInsertRecord().

One question:
The original code did not apply MAXALIGN() to SizeOfXLogRecord before
adding it. In practice SizeOfXLogRecord is likely already MAXALIGN'd
(given typical record header layout), but could you confirm whether
MAXALIGN() here is a correctness fix, a defensive no-op, or something
that requires a separate note in the commit message?

Otherwise the change looks good to me, and I think it's a reasonable
cleanup.

Tested-by: Jihyun Bahn <rring0727(at)gmail(dot)com>

Regards,
Jihyun Bahn

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ayush Tiwari 2026-04-21 04:34:11 [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION
Previous Message Michael Paquier 2026-04-21 04:23:35 Re: Typos in the code and README