| From: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
|---|---|
| To: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Some clean-ups for XLogReadDetermineTimeline() & read_local_xlog_page_guts() |
| Date: | 2026-10-23 12:42:50 |
| Message-ID: | CABPTF7XMC5oDAk6qM=cgTC_-sO7QECVoZOMbe52w=+qJVTA48A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi hackers,
Here're some proposed clean-ups for a few small issues noted while working
on [1] [2].
1) Clarify some comments
The first patch updates comments in two functions to match the existing
code.
It separates the reason for rechecking the timeline while waiting for WAL
from the .partial rename race. A cascading standby can follow an upstream
timeline change while remaining in recovery; the rename race occurs during
promotion of the server running the reader. The original comment before
invoking XLogReadDetermineTimeline() seems to mix up these two.
It also clarifies the read limits(replay for standby and flush for
primary), updates currTLIValidUntil to describe the switch LSN itself
rather than the rounding boundary, and removes a misleading FIXME:
- * Setting tli to our wanted record's TLI is slightly wrong; the
- * page might begin on an older timeline if it contains a timeline
- * switch, since its xlog segment will have been copied from the
- * prior timeline. This is pretty harmless though, as nothing
- * cares so long as the timeline doesn't go backwards. We should
- * read the page header instead; FIXME someday.
This expression seems a bit confusing. In the case of timeline change, the
pages before the switch point are copied into the segment file under the
name of the new timeline. Those page headers store the old timeline, which
should not be used as the tli to open file which may have been renamed or
removed. So I think the current behavior is ok -- tli selects the proper
segment file to open, which in this case belongs to the new timeline.
2) Remove variable tli and fix a WAL timeline assertion
Assert(!XLogRecPtrIsValid(state->currTLIValidUntil) ||
wantPage + wantLength < state->currTLIValidUntil);
We have this assertion after the slow path of reading timeline history. The
second part of seems slightly overstrict:
wantPage + wantLength < state->currTLIValidUntil
wantPage + wantLength points just passed the last byte being read.
currTLIValidUntil is exclusive if it's valid. So reading the byte before
the currTLIValidUntil seems ok in the case of wantPage + wantLength
== state->currTLIValidUntil. We may want to allow the equal case.
The patch also removes the variable tli in read_local_xlog_page_guts(),
which seems a bit redundant, especially if we remove the FIXME.
"Setting tli to our wanted record's TLI is slightly wrong"
Instead it pass the selected timeline state->currTLI directly to the
WALRead() function.
[1]
https://www.postgresql.org/message-id/CABPTF7X3JtXNxaQaTu=XNem9Qdj46Y2xsQq-WpTvfk1FqAUGGA@mail.gmail.com
[2]
https://www.postgresql.org/message-id/CABPTF7W0s_XHGkbsWdVaGs5sBefePXmG64i3RY0FsdU=b_1xwA@mail.gmail.com
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0002-Fix-a-WAL-timeline-assertion-and-simplify-timelin.patch | application/octet-stream | 3.3 KB |
| v1-0001-Clarify-comments-about-choosing-a-WAL-timeline.patch | application/octet-stream | 7.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | David Steele | 2026-10-23 12:42:13 | Re: Return pg_control from pg_backup_stop(). |