Re: corruption of WAL page header is never reported

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.
*/
- if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf))
+ if (StandbyMode &&
+ !XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf))

--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/