Hi hackers,
While working on [1], it has been noticed that pgstat_flush_io() is called for
the walwriter. Indeed, it's coming from the pgstat_report_wal() call in
WalWriterMain(). That can not report any I/O stats activity (as the
walwriter is not part of the I/O stats tracking, see pgstat_tracks_io_bktype()).
The behavior is there since 28e626bde00 and I did not find any explicit reason
to do so provided in the linked thread [2].
Calling pgstat_flush_io() from there looks unnecessary, so $SUBJECT, until the
walwriter is part of the I/O stats tracking system.
[1]:
https://www.postgresql.org/message-id/Z1rs/j5JMdTbUIJJ%40ip-10-97-1-34.eu-west-3.compute.internal
[2]:
https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
Looking forward to your feedback,
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
>From dc0ac18fb1c7720567f2fc66e146620be07f0537 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot...@gmail.com>
Date: Wed, 18 Dec 2024 12:23:56 +0000
Subject: [PATCH v1] Removing the pgstat_flush_io() call from the walwriter
pgstat_flush_io() is called for the walwriter (pgstat_report_wal() call in
WalWriterMain()). That can not report any I/O stats activity (as the
walwriter is not part of the I/O stats tracking, see pgstat_tracks_io_bktype()).
The behavior is there since 28e626bde00 and there is no any explicit reasons
to do so provided in the linked thread.
Moving pgstat_flush_io() out of pgstat_report_wal() then and don't call it in
WalWriterMain(). As a consequence, move out pgstat_flush_wal() too and remove
pgstat_report_wal().
---
src/backend/postmaster/bgwriter.c | 5 +++-
src/backend/postmaster/checkpointer.c | 7 ++++--
src/backend/postmaster/walwriter.c | 9 +++++--
src/backend/utils/activity/pgstat_wal.c | 31 ++-----------------------
src/include/pgstat.h | 1 -
5 files changed, 18 insertions(+), 35 deletions(-)
39.2% src/backend/postmaster/
58.1% src/backend/utils/activity/
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 0f75548759..7a55119e52 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -48,6 +48,7 @@
#include "storage/smgr.h"
#include "storage/standby.h"
#include "utils/memutils.h"
+#include "utils/pgstat_internal.h"
#include "utils/resowner.h"
#include "utils/timestamp.h"
@@ -235,7 +236,9 @@ BackgroundWriterMain(char *startup_data, size_t startup_data_len)
/* Report pending statistics to the cumulative stats system */
pgstat_report_bgwriter();
- pgstat_report_wal(true);
+ pgstat_flush_wal(false);
+ pgstat_flush_io(false);
+
if (FirstCallSinceLastCheckpoint())
{
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 982572a75d..9b51d4d22d 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -58,6 +58,7 @@
#include "storage/spin.h"
#include "utils/guc.h"
#include "utils/memutils.h"
+#include "utils/pgstat_internal.h"
#include "utils/resowner.h"
@@ -529,7 +530,8 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
/* Report pending statistics to the cumulative stats system */
pgstat_report_checkpointer();
- pgstat_report_wal(true);
+ pgstat_flush_wal(false);
+ pgstat_flush_io(false);
/*
* If any checkpoint flags have been set, redo the loop to handle the
@@ -607,7 +609,8 @@ HandleCheckpointerInterrupts(void)
PendingCheckpointerStats.num_requested++;
ShutdownXLOG(0, 0);
pgstat_report_checkpointer();
- pgstat_report_wal(true);
+ pgstat_flush_wal(false);
+ pgstat_flush_io(false);
/* Normal exit from the checkpointer is here */
proc_exit(0); /* done */
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 5a3cb89465..dfcda9990f 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -60,6 +60,7 @@
#include "storage/smgr.h"
#include "utils/hsearch.h"
#include "utils/memutils.h"
+#include "utils/pgstat_internal.h"
#include "utils/resowner.h"
@@ -250,8 +251,12 @@ WalWriterMain(char *startup_data, size_t startup_data_len)
else if (left_till_hibernate > 0)
left_till_hibernate--;
- /* report pending statistics to the cumulative stats system */
- pgstat_report_wal(false);
+ /*
+ * Report pending statistics to the cumulative stats system. IO
+ * statistics are not reported as not tracked for the walwriter (see
+ * pgstat_tracks_io_bktype()).
+ */
+ pgstat_flush_wal(true);
/*
* Sleep until we are signaled or WalWriterDelay has elapsed. If we
diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c
index e1d371ba3c..5de29b90c0 100644
--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -25,40 +25,13 @@ PgStat_PendingWalStats PendingWalStats = {0};
/*
* WAL usage counters saved from pgWalUsage at the previous call to
- * pgstat_report_wal(). This is used to calculate how much WAL usage
- * happens between pgstat_report_wal() calls, by subtracting
+ * pgstat_flush_wal(). This is used to calculate how much WAL usage
+ * happens between pgstat_flush_wal() calls, by subtracting
* the previous counters from the current ones.
*/
static WalUsage prevWalUsage;
-/*
- * Calculate how much WAL usage counters have increased and update
- * shared WAL and IO statistics.
- *
- * Must be called by processes that generate WAL, that do not call
- * pgstat_report_stat(), like walwriter.
- *
- * "force" set to true ensures that the statistics are flushed; note that
- * this needs to acquire the pgstat shmem LWLock, waiting on it. When
- * set to false, the statistics may not be flushed if the lock could not
- * be acquired.
- */
-void
-pgstat_report_wal(bool force)
-{
- bool nowait;
-
- /* like in pgstat.c, don't wait for lock acquisition when !force */
- nowait = !force;
-
- /* flush wal stats */
- pgstat_flush_wal(nowait);
-
- /* flush IO stats */
- pgstat_flush_io(nowait);
-}
-
/*
* Support function for the SQL-callable pgstat* functions. Returns
* a pointer to the WAL statistics struct.
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index ebfeef2f46..5194aa863d 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -772,7 +772,6 @@ extern void pgstat_execute_transactional_drops(int ndrops, struct xl_xact_stats_
* Functions in pgstat_wal.c
*/
-extern void pgstat_report_wal(bool force);
extern PgStat_WalStats *pgstat_fetch_stat_wal(void);
--
2.34.1
