git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 8741e48)
Make XLogFlush() and XLogNeedsFlush() decision-making more consistent
2025年9月19日 04:47:28 +0000 (13:47 +0900)
2025年9月19日 04:47:28 +0000 (13:47 +0900)
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 <melanieplageman@gmail.com>
Co-authored-by: Dilip Kumar <dilipbalaut@gmail.com>
Reviewed-by: Jeff Davis <pgsql@j-davis.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
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 0baf0ac6160afb53b1cb3412a6d1406e85759562..eac1de75ed0adfac238dff3da22587b8e7341730 100644 (file)
--- 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
This is the main PostgreSQL git repository.
RSS Atom

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