| From: | Paul Kim <mok03127(at)gmail(dot)com> |
|---|---|
| To: | bharath(dot)rupireddyforpostgres(at)gmail(dot)com |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Be strict when request to flush past end of WAL in WaitXLogInsertionsToFinish |
| Date: | 2026-10-13 09:26:40 |
| Message-ID: | 179188360033.12353.12948809969569320195@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Oct 9, 2026, Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> How about adjusting EndPos itself to the start of the segment (before
> the page header) using XLogBytePosToEndRecPtr in this case, where the
> switch WAL record falls right on the segment boundary, something like
> the attached?
Agreed, that fixes it where the bad position is produced. Let's go
with yours and drop my XLogBackgroundFlush() change.
0001 is your patch, with you as the author and the code unchanged. I
changed two things in the commit message:
- Your text says the walwriter keeps logging the message. That happens
with my v4, but not on master without any of these patches. There,
if the page at the boundary hasn't been initialized yet, the
walwriter PANICs in XLogWrite():
LOG: request to flush past end of generated WAL; request 0/00900028, current position 0/00900000
PANIC: xlog write request 0/00900000 is past end of log 0/00000000
and the postmaster restarts the cluster. This happens in non-assert
builds too. If the page has been initialized, it reports write and
flush positions for WAL that was never written, as your text said,
and an assert-enabled build fails Assert(Insert >= Write).
- I replaced "More blah." with a note that pg_switch_wal() now returns
what func-admin.sgml says: "the ending write-ahead log location plus
1 within the just-completed write-ahead log file". The boundary + 40
is in the next file.
0002 is a new test. The v4 test found the window by the overridden
EndPos, which your patch no longer produces, so this one records the
insert position right before pg_switch_wal() and checks that the switch
reports exactly the boundary. If a concurrent record throws off the
padding, it retries. Without 0001 it fails (boundary + 40, or + 36 on
32-bit). With 0001 it passed 30 runs in a row, and the regress,
recovery and test_misc suites pass.
Back branches: the code block is the same on REL_15 through REL_19.
0001 applies cleanly to REL_19, and with git am -3 to REL_15..18
(only the trailing context differs). On every branch the test fails
without 0001 and passes with it. The effect without the fix differs,
though:
- 17 and later: one pg_switch_wal() ending on the boundary kills the
walwriter, by the PANIC above or by the Assert, which came with
logInsertResult in 17.
- 15 and 16: I couldn't make it crash. The walwriter logs the message
once and reports write and flush positions 40 bytes past the end of
generated WAL.
I'd back-patch to all of them, though the case for 15 and 16 is
weaker. The test can keep its number; only the meson.build list needs
adjusting, and REL_15 has no meson.build there.
Side effects of 0001:
- A switch record ending on a page boundary in the middle of a segment
also used to get SizeOfXLogShortPHD added; now it reports the page
boundary. That was harmless before. The new code comment mentions
only the segment boundary, so it might be worth covering this too.
- CheckArchiveTimeout() now gets offset 0 for a switch that ends on the
boundary, which looks like "nothing happened", so its DEBUG1 message
is skipped.
- pg_walfile_name_offset(pg_switch_wal()) gives offset 0 instead of 40.
On REL_15 and REL_16, which use XLByteToPrevSeg there, the file name
also changes from the next segment to the just-completed one. That
is what their documentation describes for a location on a boundary.
I'll update the CF entry's title to match.
Regards,
Paul Kim
| Attachment | Content-Type | Size |
|---|---|---|
| v6-0001-Fix-end-LSN-reported-for-an-xlog-switch-record-en.patch | text/x-patch | 2.5 KB |
| v6-0002-Add-a-test-for-the-end-LSN-of-an-xlog-switch-reco.patch | text/x-patch | 5.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-10-13 09:41:49 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Hayato Kuroda (Fujitsu) | 2026-10-13 09:17:10 | RE: Bug in logical decoding with DDL and subtransactions |