On 2021/09/06 3:11, Alvaro Herrera wrote:
On 2021-Sep-03, Kyotaro Horiguchi wrote:diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 24165ab03e..b621ad6b0f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -12496,9 +12496,21 @@ retry: * * Validating the page header is cheap enough that doing it twice * shouldn't be a big deal from a performance point of view. + * + * Don't call XLogReaderValidatePageHeader here while not in standby mode + * so that this function won't return with a valid errmsg_buf. */ - if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf)) + if (StandbyMode && + !XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf))OK, but I don't understand why we have a comment that says (referring to non-standby mode) "doing it twice shouldn't be a big deal", followed by "Don't do it twice while not in standby mode" -- that seems quite contradictory. I think the new comment should overwrite the previous one, something like this: - * Validating the page header is cheap enough that doing it twice - * shouldn't be a big deal from a performance point of view. + * + * We do this in standby mode only, + * so that this function won't return with a valid errmsg_buf.
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. ------------------------- Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
