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/