Re: pg_walinspect: add functions to locate and list WAL by time and LSN

Hi Chao,

Thanks for the quick patch on v2, and for splitting the patch. I checked
the changes against my earlier comments, point 1 is fixed (end_lsn is now
capped before point_lookup is set), also point 2 (the invalid startLSN
message), the timeline TAP test (with archive_mode both off and on), the
test robustness changes, the move of GetXLogRecordTimestamp() to
xlogreader.c, and all of the minor items.

v2 applies cleanly to master (10b6e2a3d66) and builds without warnings, and
the regression and TAP tests pass.

0001 (pg_get_wal_files) is close. One edge case from reading the code,
GetWalTimeSegments() skips segments that start at or after the current LSN.
So if the flush LSN sits exactly on a segment boundary, as on an idle
server right after pg_switch_wal(), pg_get_wal_files(pg_current_wal_lsn())
becomes
a point lookup on a filtered-out segment and reports "WAL segment needed
for the requested range is missing".

For 0002, I think the change of boundary semantics breaks the main use
case. Since v2, start_lsn is the earliest timestamped record found at or
after the
lower bound, and end_lsn is the latest found at or before the upper bound.
But a transaction's data records come before its commit record in WAL.
So when the commit of the transaction of interest is the first timestamped
record in the window, start_lsn is that commit, and the change itself falls
outside the returned range.

I reproduced this with a scratch TAP test: autovacuum off, and no other
commits within two seconds of the window.

SELECT clock_timestamp() AS window_start \gset
BEGIN;
SELECT pg_current_wal_insert_lsn() AS delete_lsn \gset
DELETE FROM important WHERE id = 5;
SELECT pg_sleep(0.2);
COMMIT;
SELECT clock_timestamp() AS window_end \gset

SELECT l.start_lsn, l.end_lsn, :'delete_lsn'::pg_lsn AS delete_lsn
FROM pg_get_wal_location_at_time(:'window_start',
interval '500 milliseconds',
:'window_end'::timestamptz -
:'window_start'::timestamptz) AS l;

start_lsn | end_lsn | delete_lsn
------------+------------+------------
0/017EEF78 | 0/017EEF78 | 0/017EEF40

The DELETE ran inside the requested window, and its WAL begins at
0/017EEF40, the insert position taken just before it. Its commit, 56 bytes
later, is the only timestamped record in the window, so it's returned as
both boundaries. Following the documented workflow,
pg_get_wal_records_info(start_lsn, end_lsn) then returns no rows at all,
because that function only emits records that end at or before end_lsn. So
the user sees neither the DELETE nor its commit. On a quiet system, which
is
when someone is most likely to be investigating a single mistaken
statement,
this is the usual outcome, not a corner case.

Outer anchors (the latest timestamped record at or before the lower bound,
and the earliest at or after the upper bound) avoid this, but only with a
fallback for the end. In this reproducer nothing commits after the window,
so by my reading v1 would have failed with "requested time follows the
available WAL range" instead. pg_walinspect already treats the current WAL
position as a valid end (ValidateInputLSNs() caps end_lsn to it, and the
docs suggest FFFFFFFF/FFFFFFFF for exactly that), so falling back to the
current flush/replay LSN, with end_timestamp NULL, seems consistent with the
rest of the module. It would also cover the quiet-system case from my first
review.

I don't think the binary search requires inner anchors. It can locate the
segment for each bound and then take the outer record from that segment, or
from the nearest earlier (or later) segment that has a timestamped record.
Outer anchors still won't cover a transaction that began before the lower
anchor, but the docs already say as much, and it is a much narrower case.

Separately, reading the code, a window that contains no timestamped record,
but has some on both sides, currently ends in "could not find a valid WAL
range for the requested time window" with the detail "The matching lower
boundary follows the upper boundary in WAL". That reads like a WAL ordering
problem, when the real cause is that nothing in the window is timestamped.
With outer anchors this case would just return a range.

Regards,
Surya Poondla