From d02d851ab515d4a1f883f657bc43a9cae600a5d8 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 29 Aug 2022 19:56:27 +1200 Subject: [PATCH v2 1/3] Fix cache invalidation in rare recovery_prefetch cases. XLogPageRead() can retry internally in rare cases after a read has succeeded, overwriting the page buffer. Namely, short reads, and page validation failures while in standby mode (see commit 0668719801). Due to an oversight in commit 3f1ce973, these cases could leave stale data in the internal cache of xlogreader.c without marking it invalid. The main defense against stale cached data was in the error handling path of ReadPageInternal(), but that wasn't quite enough for errors handled internally by XLogPageRead()'s retry loop. Instead of trying to invalidate the cache in the relevant failure cases, ReadPageInternal() now invalidates it before even calling the page_read callback (ie XLogPageRead()). It is only marked valid by setting state->readLen etc after page_read() succeeds. It remains valid until the caller of ReadPageInternal() eventually asks for something out of range and we have to go back to page_read() for more. The removed line that was setting errormsg_deferred was redundant because it's already set by report_invalid_record(). Back-patch to 15. Reported-by: Noah Misch Reviewed-by: Discussion: https://postgr.es/m/20220807003627.GA4168930%40rfd.leadboat.com --- src/backend/access/transam/xlogreader.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index f17e80948d..e883ade607 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -987,6 +987,13 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) targetPageOff == state->segoff && reqLen <= state->readLen) return state->readLen; + /* + * Invalidate contents of internal buffer before read attempt. Just set + * the length to 0, rather than a full XLogReaderInvalReadState(), so we + * don't forget the segment we last read. + */ + state->readLen = 0; + /* * Data is not in our buffer. * @@ -1067,11 +1074,6 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) return readLen; err: - if (state->errormsg_buf[0] != '\0') - { - state->errormsg_deferred = true; - XLogReaderInvalReadState(state); - } return XLREAD_FAIL; } -- 2.30.2