>Original
>From: cca5507 <[email protected]>
>Date: 2026-04-21 19:35
>To:반지현<[email protected]>, pgsql-hackers <[email protected]>
>Subject: Re: Return value of XLogInsertRecord() for XLOG_SWITCH record
>
>
>Hi, thanks for the test!
>
>> 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.
>
>I apply the MAXALIGN() to keep it consistent with ReserveXLogSwitch(), the
>value seems unchanged though.
>
>Attach v2 which is more efficient.
>
>--
>Regards,
>ChangAo Chen


Hi, cca5507,반지현,

Thanks for your patch and test— it’s sparked my deeper dive into the WAL switch 
logic.

I’ve done a preliminary survey of all top-level callers of XLogInsertRecord() 
that generate XLOG_SWITCH records, and grouped them into three categories:
1.do_pg_backup_start(), do_pg_backup_stop(), ShutdownXLOG()
    These callers completely ignore the return value of XLogInsertRecord(), 
so a semantic change here would have zero internal impact on them.

2.CheckArchiveTimeout()
  It stores the returned LSN into switchpoint only for debug logging:
```c
if (XLogSegmentOffset(switchpoint, wal_segment_size) != 0)
    elog(DEBUG1, "write-ahead log switch forced (\"archive_timeout\"=%d)", 
    XLogArchiveTimeout);
```c
The offset check here is just a trivial debug print, and would not 
introduce functional defects even if the returned LSN changes its meaning.

3.pg_switch_wal(PG_FUNCTION_ARGS) — the critical public entry point
    This function directly returns the LSN from XLogInsertRecord() as 
its SQL-level return value exposed to end users, scripts,  and external tooling.

Risk analysis:
From the above classification, internal PostgreSQL core logic suffers 
no functional breakage if we unify the return value semantics of 
XLogInsertRecord().

However, there are compatibility risks for external consumers relying on 
pg_switch_wal()’s output:
Custom applications, backup scripts, and monitoring tools may take 
the returned LSN as the exact position of the XLOG_SWITCH record to 
run space / time range analysis between the switch record and segment end.

Since we cannot guarantee how existing third-party systems interpret this 
return 
LSN, altering its definition would break established external workflows and 
trigger 
unpredictable side effects for legacy deployments.

That is the main backward-compatibility risk I can identify for this change.

Happy to hear your thoughts or any corrections to my analysis.


regards,
--
ZizhuanLiu (X-MAN) 
[email protected]

Reply via email to