On 2023-09-25 09:56, Michael Paquier wrote:
It seems to me that you are right here. It would make sense to me to
say that force=true is equivalent to nowait=false, as in "I'm OK to
wait on the lockas I want to make sure that the stats are flushed at
this point". Currently force=true means nowait=true, as in "I'm OK to
not have the stats flushed if I cannot take the lock".
Seeing the three callers of pgstat_report_wal(), the checkpointer
wants to force its way twice, and the WAL writer does not care if they
are not flushed immediately at it loops forever in this path.
A comment at the top of pgstat_report_wal() would be nice to document
that a bit better, at least.
Thank you for the review. Certainly, adding a comments is a good idea. I
added a comment.
Ryoga Yoshida
diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c
index bcaed14d02..6f3bac5d0f 100644
--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -38,13 +38,18 @@ static WalUsage prevWalUsage;
*
* Must be called by processes that generate WAL, that do not call
* pgstat_report_stat(), like walwriter.
+ *
+ * force=false is the same as nowait=true, and the statistics will
+ * not be flushed if the lock cannot be acquired.
+ * force=true is the same as nowait=false, and waits until the lock
+ * is acquired and ensures that the statistics are flushed.
*/
void
pgstat_report_wal(bool force)
{
- pgstat_flush_wal(force);
+ pgstat_flush_wal(!force);
- pgstat_flush_io(force);
+ pgstat_flush_io(!force);
}
/*
