Hi Tomas,
Thanks for the patch. I went through it and the fix looks right to me.
I also looked at the other callers of XLogPrefetcherBeginRead(), and I
agree with the commit message: apart from FinishWalRecovery(), each of them
runs after at most one
record has been read and returned, so prefetcher->record is already NULL
there.
The reason this goes unnoticed on non-assert builds is that
FinishWalRecovery() throws away the result of ReadRecord(). Even with emode
= PANIC, ReadRecord() just returns NULL quietly when there's no errormsg,
and EndRecPtr is still the start of the record, which matches what you
describe. I couldn't find a case where re-reading the last replayed record
is expected to fail, so maybe it's worth adding a check there, probably as
a separate patch:
XLogPrefetcherBeginRead(xlogprefetcher, lastRec);
if (ReadRecord(xlogprefetcher, PANIC, false, lastRecTLI) == NULL)
elog(PANIC, "could not re-read last replayed record at
%X/%08X",
LSN_FORMAT_ARGS(lastRec));
That way, if something like this ever happens again, it would fail loudly.
Regards,
Surya Poondla
