On Wed, Oct 28, 2026, surya poondla <[email protected]> wrote: > I looked at v7. 0001 looks right to me, [...] > I have a few comments:
Thanks for the review. v8 attached; all four points are in 0001, and 0002 is unchanged. > 1. This can also happen during backups, not just with pg_switch_wal(). Right, the commit message now mentions the backup functions too. > 2. The new comment only talks about the segment boundary, [...] Taken, with your wording. > 3. [...] CheckArchiveTimeout() says "If the returned pointer points > exactly to a segment boundary, assume nothing happened." I kept the check and extended the comment. Telling the two cases apart would need a new API, which seems too much for a DEBUG1 message. > 4. [...] adding Assert(EndPos <= reservedEnd) after the new > computation [...] Added. Regards, Paul Kim
From e32c7550a594b0c15e68e62ad2afb4541338852d Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Fri, 9 Oct 2026 04:16:25 +0000 Subject: [PATCH v8 1/2] Fix end LSN reported for an xlog-switch record ending on a page boundary A page header size is added to the end of an xlog-switch record even when the record ends exactly on a page boundary and crosses no header. At a segment boundary that puts the end past the end of reserved WAL. A backend that requested the switch (pg_switch_wal(), or do_pg_backup_start() and do_pg_backup_stop() during a backup) then commits without an XID, so the position goes into the asynchronous commit LSN, and from there into the WAL writer, which asks to write and flush WAL that was never generated. If the page at the boundary has not been initialized yet, XLogWrite() then PANICs with "xlog write request ... is past end of log". Otherwise it reports write and flush positions for WAL that was never written, and in an assert-enabled build fails an assertion there. Compute the end of the record with XLogBytePosToEndRecPtr(), the way it is done for any other record, so that a record ending exactly on a page boundary points to the boundary, and assert that it stays within the reserved WAL. Also note in CheckArchiveTimeout() that a switch record ending exactly on a segment boundary now looks like no switch happened there, which only skips a debug message. This also makes pg_switch_wal() return what its documentation says: the ending location plus 1 within the just-completed WAL file. --- src/backend/access/transam/xlog.c | 18 ++++++++---------- src/backend/postmaster/checkpointer.c | 4 +++- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 8636ad9ab2c..25ffc0a1e0c 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1077,20 +1077,18 @@ XLogInsertRecord(XLogRecData *rdata, /* * Even though we reserved the rest of the segment for us, which is * reflected in EndPos, we return a pointer to just the end of the - * xlog-switch record. + * xlog-switch record. Compute it the same way as for any other + * record, so that a record ending exactly on a page boundary points + * to the boundary rather than past the next page header. On a + * segment boundary, the latter would be beyond the WAL we reserved. */ if (inserted) { - EndPos = StartPos + SizeOfXLogRecord; - if (StartPos / XLOG_BLCKSZ != EndPos / XLOG_BLCKSZ) - { - uint64 offset = XLogSegmentOffset(EndPos, wal_segment_size); + XLogRecPtr reservedEndPos PG_USED_FOR_ASSERTS_ONLY = EndPos; - if (offset == EndPos % XLOG_BLCKSZ) - EndPos += SizeOfXLogLongPHD; - else - EndPos += SizeOfXLogShortPHD; - } + EndPos = XLogBytePosToEndRecPtr(XLogRecPtrToBytePos(StartPos) + + MAXALIGN(SizeOfXLogRecord)); + Assert(EndPos <= reservedEndPos); } } diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 02e4f6c338a..1761053ea2e 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -744,7 +744,9 @@ CheckArchiveTimeout(void) /* * If the returned pointer points exactly to a segment boundary, - * assume nothing happened. + * assume nothing happened. This is also what we get when a + * switch record happens to end exactly on the boundary, but it + * only costs us the debug message. */ if (XLogSegmentOffset(switchpoint, wal_segment_size) != 0) elog(DEBUG1, "write-ahead log switch forced (\"archive_timeout\"=%d)", -- 2.50.1 (Apple Git-155)
From f453e2a47b9d583ae3bd509cbea44555380b3173 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Fri, 23 Oct 2026 20:12:35 +0900 Subject: [PATCH v8 2/2] Add a test for the end LSN of an xlog-switch record An xlog-switch record that starts SizeOfXLogRecord bytes before a segment boundary ends exactly on it, and pg_switch_wal() must report the boundary. Without the fix, it reports the boundary plus SizeOfXLogLongPHD. The test goes into 039_end_of_wal.pl, whose node is set up so that nothing else writes WAL. It pads the segment with logical messages, measuring the record overhead and page header size at run time instead of hard-coding them. --- src/test/recovery/t/039_end_of_wal.pl | 69 +++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/test/recovery/t/039_end_of_wal.pl b/src/test/recovery/t/039_end_of_wal.pl index 5fe8835e094..fa4ca73fd73 100644 --- a/src/test/recovery/t/039_end_of_wal.pl +++ b/src/test/recovery/t/039_end_of_wal.pl @@ -376,4 +376,73 @@ ok( $node->log_contains( "invalid contrecord length 123456 .* at .*", $log_size), "xlp_rem_len bad (split record header)"); + +########################################################################### +note "xlog-switch record ending on a segment boundary"; +########################################################################### + +# A switch record that starts SizeOfXLogRecord bytes before the end of a +# segment ends exactly on the boundary, and pg_switch_wal() has to return +# the boundary, not a position past the next page header. + +# Emit a non-transactional logical message (no XID, no commit record) +# with a payload of $size bytes, returning its end LSN in bytes. +sub emit_message +{ + my ($node, $size) = @_; + return int( + $node->safe_psql( + 'postgres', + "SELECT pg_logical_emit_message(false, '', repeat('a', $size)) - '0/0'" + )); +} + +my $SizeOfXLogRecord = 24; + +# Start on a fresh segment. Nothing else writes WAL on this node, so the +# positions below are exact. +$node->safe_psql('postgres', "SELECT pg_switch_wal()"); +my $lsn = int( + $node->safe_psql( + 'postgres', "SELECT pg_current_wal_insert_lsn() - '0/0'")); +my $seg_end = ($lsn / $WAL_SEGMENT_SIZE + 1) * $WAL_SEGMENT_SIZE; + +# Measure the space a message takes beyond its payload, padding included. +# 256 is a multiple of MAXALIGN, over 255 so the record uses the long +# main-data header like the fill, and small enough to stay on the first page. +my $probe_size = 256; +my $end = emit_message($node, $probe_size); +my $overhead = $end - $lsn - $probe_size; +$lsn = $end; + +# Measure the page header size with a message crossing one page boundary. +# The first page of a segment has a longer header, but nothing below +# crosses a segment boundary. +my $size = start_of_next_page($lsn) - $lsn; +$end = emit_message($node, $size); +my $SizeOfXLogShortPHD = $end - $lsn - $size - $overhead; +$lsn = $end; + +# Fill the segment in steps, so that no record gets too big for any +# segment size, until the rest fits in one message. +my $step = $WAL_SEGMENT_SIZE / 16; +$lsn = emit_message($node, $step) while $seg_end - $lsn > 2 * $step; + +# Size the last message to end SizeOfXLogRecord bytes before the +# boundary, less the page headers it crosses. A record ending exactly on +# a page boundary reports the boundary while the next one starts after +# the page header, hence the - 1. +my $headers = + ($seg_end - start_of_next_page($lsn - 1)) / $WAL_BLOCK_SIZE * + $SizeOfXLogShortPHD; +$lsn = emit_message($node, + $seg_end - $SizeOfXLogRecord - $lsn - $headers - $overhead); +is($lsn, $seg_end - $SizeOfXLogRecord, + "message ends SizeOfXLogRecord bytes before the segment end"); + +# The switch record is only a header, so it ends on the boundary. +is( $node->safe_psql('postgres', "SELECT pg_switch_wal() - '0/0'"), + $seg_end, + "pg_switch_wal() returns the segment boundary"); + done_testing(); -- 2.50.1 (Apple Git-155)
