From 00c6f37eb802bb902ad4f79cf299309e661766c5 Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Fri, 12 Sep 2025 08:37:15 +0530 Subject: [PATCH v2] 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 | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0baf0ac6160..712a41d7879 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -2938,6 +2938,12 @@ 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)); + + /* + * Assert that a flush is no longer needed after the flush has completed + * for this record. + */ + Assert(!XLogNeedsFlush(record)); } /* @@ -3113,9 +3119,11 @@ 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. + * would need to be updated. The end-of-recovery checkpoint still must + * flush WAL like normal, though, so check !XLogInsertAllowed(). This + * check must be consistent with XLogFlush(). */ - if (RecoveryInProgress()) + if (!XLogInsertAllowed()) { /* * An invalid minRecoveryPoint means that we need to recover all the -- 2.51.0.384.g4c02a37b29-goog