diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index ebd86a79ff39581d1e7d18aa2f349302e430d25d..1517f78aec5739c042ff2e4cff5fc2663b1bfbb3 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -10988,6 +10988,40 @@ retry: Assert(reqLen <= readLen); *readTLI = curFileTLI; + + /* + * Check the page header immediately, so that we can retry immediately if + * it's not valid. This may seem unnecessary, because XLogReadRecord() + * validates the page header anyway, and would propagate the failure up to + * ReadRecord(), which would retry. However, there's a corner case with + * continuation records, if a record is split across two pages such that + * we would need to read the two pages from different sources. For + * example, imagine a scenario where a streaming replica is started up, + * and replay reaches a record that's split across two WAL segments. The + * first page is only available locally, in pg_wal, because it's already + * been recycled in the master. The second page, however, is not present + * in pg_wal, and we should stream it from the master. There is a recycled + * WAL segment present in pg_wal, with garbage contents, however. We would + * read the first page from the local WAL segment, but when reading the + * second page, we would read the bogus, recycled, WAL segment. If we + * didn't catch that case here, we would never recover, because + * ReadRecord() would retry reading the whole record from the beginning. + * + * Of course, this only catches errors in the page header, which is what + * happens in the case of a recycled WAL segment. Other kinds of errors or + * corruption still has the same problem. But this at least fixes the + * common case, which can happen as part of normal operation. + * + * Validating the page header is cheap enough that doing it twice + * shouldn't be a big deal from a performance point of view. + */ + if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf)) + { + /* reset any error XLogReaderValidatePageHeader() might have set */ + xlogreader->errormsg_buf[0] = '\0'; + goto next_record_is_invalid; + } + return readLen; next_record_is_invalid: @@ -11121,12 +11155,18 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, } else { - ptr = tliRecPtr; + ptr = RecPtr; + + /* + * Use the record begin position to determine the + * TLI, rather than the position we're reading. + */ tli = tliOfPointInHistory(tliRecPtr, expectedTLEs); if (curFileTLI > 0 && tli < curFileTLI) elog(ERROR, "according to history file, WAL location %X/%X belongs to timeline %u, but previous recovered WAL file came from timeline %u", - (uint32) (ptr >> 32), (uint32) ptr, + (uint32) (tliRecPtr >> 32), + (uint32) tliRecPtr, tli, curFileTLI); } curFileTLI = tli; diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index e9a8068e31bd2c9e023fd32090c255757f00d2eb..4c28ea64e3b8c6bea5745f0631076db1c42efd16 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -23,8 +23,6 @@ static bool allocate_recordbuf(XLogReaderState *state, uint32 reclength); -static bool ValidXLogPageHeader(XLogReaderState *state, XLogRecPtr recptr, - XLogPageHeader hdr); static bool ValidXLogRecordHeader(XLogReaderState *state, XLogRecPtr RecPtr, XLogRecPtr PrevRecPtr, XLogRecord *record, bool randAccess); static bool ValidXLogRecord(XLogReaderState *state, XLogRecord *record, @@ -500,7 +498,6 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) */ if (targetSegNo != state->readSegNo && targetPageOff != 0) { - XLogPageHeader hdr; XLogRecPtr targetSegmentPtr = pageptr - targetPageOff; readLen = state->read_page(state, targetSegmentPtr, XLOG_BLCKSZ, @@ -512,9 +509,8 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) /* we can be sure to have enough WAL available, we scrolled back */ Assert(readLen == XLOG_BLCKSZ); - hdr = (XLogPageHeader) state->readBuf; - - if (!ValidXLogPageHeader(state, targetSegmentPtr, hdr)) + if (!XLogReaderValidatePageHeader(state, targetSegmentPtr, + state->readBuf)) goto err; } @@ -551,7 +547,7 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) /* * Now that we know we have the full header, validate it. */ - if (!ValidXLogPageHeader(state, pageptr, hdr)) + if (!XLogReaderValidatePageHeader(state, pageptr, (char *) hdr)) goto err; /* update cache information */ @@ -751,15 +747,19 @@ ValidXLogRecord(XLogReaderState *state, XLogRecord *record, XLogRecPtr recptr) } /* - * Validate a page header + * Validate a page header. + * + * Check if 'phdr' is valid as the header of the XLog page at position + * 'recptr'. */ -static bool -ValidXLogPageHeader(XLogReaderState *state, XLogRecPtr recptr, - XLogPageHeader hdr) +bool +XLogReaderValidatePageHeader(XLogReaderState *state, XLogRecPtr recptr, + char *phdr) { XLogRecPtr recaddr; XLogSegNo segno; int32 offset; + XLogPageHeader hdr = (XLogPageHeader) phdr; Assert((recptr % XLOG_BLCKSZ) == 0); @@ -847,6 +847,11 @@ ValidXLogPageHeader(XLogReaderState *state, XLogRecPtr recptr, return false; } + /* + * Check that the address on the page agrees with what we expected. + * This check typically fails when an old WAL segment is recycled, + * and hasn't yet been overwritten with new data yet. + */ if (hdr->xlp_pageaddr != recaddr) { char fname[MAXFNAMELEN]; diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index ea873a2d9c76d68b36639ff4e3827cd61989f8ca..4474f2b2e798ae41574bb07fdb6f9218e20a62c6 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -119,6 +119,10 @@ extern void XLogReaderFree(XLogReaderState *state); extern struct XLogRecord *XLogReadRecord(XLogReaderState *state, XLogRecPtr recptr, char **errormsg); +/* Validate a page */ +extern bool XLogReaderValidatePageHeader(XLogReaderState *state, + XLogRecPtr recptr, char *phdr); + #ifdef FRONTEND extern XLogRecPtr XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr); #endif /* FRONTEND */