From 22d742c57fa19eeb4f1290ef30c1d42c8303220a Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Fri, 12 Sep 2025 08:37:15 +0530 Subject: [PATCH v3] Make XLogFlush() and XLogNeedsFlush() decision more consistent XLogFlush() and XLogNeedsFlush() previously used inconsistent criteria for determining the WAL flush status. XLogFlush() used XLogInsertAllowed() to determine whether to perform WAL flush or just update minRecoveryPoint. In contrast, XLogNeedsFlush() relied on checking RecoveryInProgress() to identify whether to check against the minRecoveryPoint or the current flush location. This difference introduced a logical inconsistency. For example, during an end-of-recovery checkpoint, the checkpointer was allowed to flush the WAL. However, XLogNeedsFlush() determined its need for a flush based on minRecoveryPoint because RecoveryInProgress() was true, leading to a reliance on the minRecoveryPoint when it was not applicable. This commit resolves the inconsistency by having XLogNeedsFlush() also base its decision on the result of XLogInsertAllowed(). To further ensure consistency, XLogFlush() adds an assertion that XLogNeedsFlush() returns false after XLogFlush() has completed its job. The inconsistency described above was not observed as a live bug, but this patch hardens the logic to prevent potential issues. --- src/backend/access/transam/xlog.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0baf0ac6160a..0fd286d2c87c 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -2938,6 +2938,13 @@ XLogFlush(XLogRecPtr record) "xlog flush request %X/%08X is not satisfied --- flushed only to %X/%08X", LSN_FORMAT_ARGS(record), LSN_FORMAT_ARGS(LogwrtResult.Flush)); + + /* + * Cross-check XLogNeedsFlush(). Some of the checks of XLogFlush() and + * XLogNeedsFlush() are duplicated, and this assertion ensures that these + * remain consistent. + */ + Assert(!XLogNeedsFlush(record)); } /* @@ -3114,8 +3121,13 @@ XLogNeedsFlush(XLogRecPtr record) * During recovery, we don't flush WAL but update minRecoveryPoint * instead. So "needs flush" is taken to mean whether minRecoveryPoint * would need to be updated. + * + * XLogInsertAllowed() is used as an end-of-recovery checkpoint is + * launched while recovery is still in progress, RecoveryInProgress() + * returning true in this case. This check should be consistent with the + * one in XLogFlush(). */ - if (RecoveryInProgress()) + if (!XLogInsertAllowed()) { /* * An invalid minRecoveryPoint means that we need to recover all the -- 2.51.0