Re: A bug in recovery prefetcher, triggering asserts / FATAL: checkpoint request failed

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