diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 34a33112af..61061e5257 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -11273,6 +11273,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: @@ -11406,12 +11440,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 4499fe0627..f950c5f46e 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -27,8 +27,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, @@ -530,7 +528,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, @@ -542,9 +539,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; } @@ -581,7 +577,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 read state information */ @@ -706,15 +702,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); @@ -802,6 +802,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 deaa7f5128..26f8bc056a 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -180,6 +180,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); + /* Invalidate read state */ extern void XLogReaderInvalReadState(XLogReaderState *state);