From 564c1d17d975a6daf44ff1a5c8017c1f5dd2e57b Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Fri, 12 Sep 2025 08:37:15 +0530 Subject: [PATCH v1] Make XLogFlush() and XLogNeedsFlush() decision more consistent Make XLogFlush() and XLogNeedsFlush() more consistent in their decision-making process for flushing the WAL. Currently, XLogFlush() uses XLogInsertAllowed() to determine whether to flush the WAL or just update minRecoveryPoint. In contrast, XLogNeedsFlush() checks RecoveryInProgress(). This can lead to inconsistencies. For example, during an end-of-recovery checkpoint, the checkpointer is allowed to flush the WAL, so XLogFlush() will proceed with the flush. However, XLogNeedsFlush() will still check RecoveryInProgress(), which returns true, leading it to mistakenly rely on minRecoveryPoint. This commit resolves the inconsistency by having XLogNeedsFlush() also base its decision on XLogInsertAllowed(). To further ensure consistency, XLogFlush() will add and assert that XLogNeedsFlush() return false after XLogFlush() is done with its job. --- src/backend/access/transam/xlog.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0baf0ac6160..dab93329e07 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)); } /* @@ -3111,11 +3117,11 @@ bool 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. + * If Xlog insert is not allowed, we don't flush WAL but update + * minRecoveryPoint instead. So "needs flush" is taken to mean whether + * minRecoveryPoint would need to be updated. */ - if (RecoveryInProgress()) + if (!XLogInsertAllowed()) { /* * An invalid minRecoveryPoint means that we need to recover all the -- 2.49.0