On 2021/09/07 2:02, Fujii Masao wrote:
Even if we do this while NOT in standby mode, ISTM that this function doesn't
return with a valid errmsg_buf because it's reset. So probably the comment
should be updated as follows?
-------------------------
We don't do this while not in standby mode because we don't need to retry
immediately if the page header is not valid. Instead, XLogReadRecord() is
responsible to check the page header.
-------------------------
I updated the comment as above. Patch attached.
- * it's not valid. This may seem unnecessary, because XLogReadRecord()
+ * it's not valid. This may seem unnecessary, because ReadPageInternal()
* validates the page header anyway, and would propagate the failure up
to
I also applied this change because ReadPageInternal() not XLogReadRecord()
calls XLogReaderValidatePageHeader().
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index e51a7a749d..35817d9d1e 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -12274,7 +12274,7 @@ retry:
/*
* Check the page header immediately, so that we can retry immediately
if
- * it's not valid. This may seem unnecessary, because XLogReadRecord()
+ * it's not valid. This may seem unnecessary, because ReadPageInternal()
* 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
@@ -12298,9 +12298,22 @@ retry:
*
* Validating the page header is cheap enough that doing it twice
* shouldn't be a big deal from a performance point of view.
+ *
+ * Note that we don't do this while not in standby mode because we don't
+ * need to retry immediately if the page header is not valid. Instead,
+ * ReadPageInternal() is responsible for validating the page header.
*/
- if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf))
+ if (StandbyMode &&
+ !XLogReaderValidatePageHeader(xlogreader, targetPagePtr,
readBuf))
{
+ /*
+ * Emit this error right now then retry this page immediately.
Use
+ * errmsg_internal() because the message was already translated.
+ */
+ if (xlogreader->errormsg_buf[0])
+ ereport(emode_for_corrupt_record(emode, EndRecPtr),
+ (errmsg_internal("%s",
xlogreader->errormsg_buf)));
+
/* reset any error XLogReaderValidatePageHeader() might have
set */
xlogreader->errormsg_buf[0] = '\0';
goto next_record_is_invalid;