From: Michael Paquier Date: 2025年9月19日 04:47:28 +0000 (+0900) Subject: Make XLogFlush() and XLogNeedsFlush() decision-making more consistent X-Git-Url: http://git.postgresql.org/gitweb/?a=commitdiff_plain;h=deb208df4559c922df217354f2c0cb689016a2ed;p=postgresql.git Make XLogFlush() and XLogNeedsFlush() decision-making more consistent When deciding which code path to use depending on the state of recovery, XLogFlush() and XLogNeedsFlush() have been relying on different criterias: - XLogFlush() relied on XLogInsertAllowed(). - XLogNeedsFlush() relied on RecoveryInProgress(). Currently, the checkpointer is allowed to insert WAL records while RecoveryInProgress() returns true for an end-of-recovery checkpoint, where XLogInsertAllowed() matters. Using RecoveryInProgress() in XLogNeedsFlush() did not really matter for its existing callers, as the checkpointer only called XLogFlush(). However, a feature under discussion, by Melanie Plageman, needs XLogNeedsFlush() to be able to work in more contexts, the end-of-recovery checkpoint being one. This commit changes XLogNeedsFlush() to use XLogInsertAllowed() instead of RecoveryInProgress(), making the checks in both routines more consistent. While on it, an assertion based on XLogNeedsFlush() is added at the end of XLogFlush(), triggered when flushing a physical position (not for the normal recovery patch that checks for updates of the minimum recovery point). This assertion would fail for example in the recovery test 015_promotion_pages if XLogNeedsFlush() is changed to use RecoveryInProgress(). This should be hopefully enough to ensure that the checks done in both routines remain consistent. Author: Melanie Plageman Co-authored-by: Dilip Kumar Reviewed-by: Jeff Davis Reviewed-by: Chao Li Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAAKRu_a1vZRZRWO3_jv_X13RYoqLRVipGO0237g5PKzPa2YX6g@mail.gmail.com --- diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0baf0ac6160..eac1de75ed0 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)); } /* @@ -3102,10 +3109,16 @@ XLogBackgroundFlush(void) } /* - * Test whether XLOG data has been flushed up to (at least) the given position. + * Test whether XLOG data has been flushed up to (at least) the given + * position, or whether the minimum recovery point has been updated past + * the given position. + * + * Returns true if a flush is still needed, or if the minimum recovery point + * must be updated. * - * Returns true if a flush is still needed. (It may be that someone else - * is already in process of flushing that far, however.) + * It is possible that someone else is already in the process of flushing + * that far, or has updated the minimum recovery point up to the given + * position. */ bool XLogNeedsFlush(XLogRecPtr record) @@ -3114,8 +3127,12 @@ 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. + * + * Using XLogInsertAllowed() rather than RecoveryInProgress() matters for + * the case of an end-of-recovery checkpoint, where WAL data is flushed. + * 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

AltStyle によって変換されたページ (->オリジナル) /