Thanks all for the comments!
On Fri, Nov 3, 2023 at 5:17 AM Matthias van de Meent
<boekewurm+postg...@gmail.com> wrote:
Knowing that your metrics have a shared starting point can be quite
valuable, as it allows you to do some math that would otherwise be
much less accurate when working with stats over a short amount of
time. I've not used these stats systems much myself, but skew between
metrics caused by different reset points can be difficult to detect
and debug, so I think an atomic call to reset all these stats could be
worth implementing.
Since each stats, except wal_prefetch was reset acquiring LWLock,
attached PoC patch makes the call atomic by using these LWlocks.
If this is the right direction, I'll try to make wal_prefetch also take
LWLock.
On 2023-11-04 10:49, Andres Freund wrote:
Yes, agreed. However, I'd suggest adding pg_stat_reset_shared(),
instead of
pg_stat_reset_shared('all') for this purpose.
In the attached PoC patch the shared statistics are reset by calling
pg_stat_reset_shared() not pg_stat_reset_shared('all').
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
From f0a5cc6ad6cc351adff9302c3e9b2a227a5d9cb4 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Mon, 6 Nov 2023 14:53:19 +0900
Subject: [PATCH v1] [WIP]Add ability to reset all statistics to
pg_stat_reset_shared()
Currently pg_stat_reset_shared() requires its argument to specify
statistics to be reset, and there is no way to reset all statistics
at the same time.
This patch makes it possible when no argument is specified.
Note this patch is WIP and pg_stat_recovery_prefetch is not reset.
---
doc/src/sgml/monitoring.sgml | 4 +-
src/backend/utils/activity/pgstat.c | 75 +++++++++++++++++++++++++++++
src/backend/utils/adt/pgstatfuncs.c | 10 ++++
src/include/catalog/pg_proc.dat | 4 ++
src/include/pgstat.h | 1 +
5 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index e068f7e247..9d1e3bf4db 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4716,7 +4716,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<returnvalue>void</returnvalue>
</para>
<para>
- Resets some cluster-wide statistics counters to zero, depending on the
+ Resets cluster-wide statistics counters to zero, depending on the
argument. The argument can be <literal>bgwriter</literal> to reset
all the counters shown in
the <structname>pg_stat_bgwriter</structname> view,
@@ -4730,6 +4730,8 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structname>pg_stat_wal</structname> view or
<literal>recovery_prefetch</literal> to reset all the counters shown
in the <structname>pg_stat_recovery_prefetch</structname> view.
+ If no argument is specified, reset all these views at once.
+ Note current patch is WIP and pg_stat_recovery_prefetch is not reset.
</para>
<para>
This function is restricted to superusers by default, but other users
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index d743fc0b28..9b9398439f 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -767,6 +767,81 @@ pgstat_reset_of_kind(PgStat_Kind kind)
pgstat_reset_entries_of_kind(kind, ts);
}
+/* Reset below cluster-wide statistics:
+ * - pg_stat_bgwriter
+ * - pg_stat_checkpointer
+ * - pg_stat_archiver
+ * - pg_stat_io
+ * - pg_stat_wal
+ *
+ * Note recovery_prefetch is not implemented(WIP).
+ */
+void
+pgstat_reset_shared_all(void)
+{
+#define STATS_SHARED_NUM_LWLOCK 4
+ TimestampTz ts = GetCurrentTimestamp();
+
+ PgStatShared_Archiver *stats_archiver = &pgStatLocal.shmem->archiver;
+ PgStatShared_BgWriter *stats_bgwriter = &pgStatLocal.shmem->bgwriter;
+ PgStatShared_Checkpointer *stats_checkpointer = &pgStatLocal.shmem->checkpointer;
+ PgStatShared_Wal *stats_wal = &pgStatLocal.shmem->wal;
+ PgStatShared_IO *stats_io = &pgStatLocal.shmem->io;
+
+ // Acquire LWLocks
+ LWLock *locks[] = {&stats_archiver->lock, &stats_bgwriter->lock,
+ &stats_checkpointer->lock, &stats_wal->lock};
+
+ for (int i = 0; i < STATS_SHARED_NUM_LWLOCK; i++)
+ LWLockAcquire(locks[i], LW_EXCLUSIVE);
+
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ {
+ LWLock *bktype_lock = &stats_io->locks[i];
+ LWLockAcquire(bktype_lock, LW_EXCLUSIVE);
+ }
+
+ // Reset stats
+ pgstat_copy_changecounted_stats(&stats_archiver->reset_offset,
+ &stats_archiver->stats,
+ sizeof(stats_archiver->stats),
+ &stats_archiver->changecount);
+ stats_archiver->stats.stat_reset_timestamp = ts;
+
+ pgstat_copy_changecounted_stats(&stats_bgwriter->reset_offset,
+ &stats_bgwriter->stats,
+ sizeof(stats_bgwriter->stats),
+ &stats_bgwriter->changecount);
+ stats_bgwriter->stats.stat_reset_timestamp = ts;
+
+ pgstat_copy_changecounted_stats(&stats_checkpointer->reset_offset,
+ &stats_checkpointer->stats,
+ sizeof(stats_checkpointer->stats),
+ &stats_checkpointer->changecount);
+ stats_checkpointer->stats.stat_reset_timestamp = ts;
+
+ memset(&stats_wal->stats, 0, sizeof(stats_wal->stats));
+ stats_wal->stats.stat_reset_timestamp = ts;
+
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ {
+ PgStat_BktypeIO *bktype_shstats = &stats_io->stats.stats[i];
+ memset(bktype_shstats, 0, sizeof(*bktype_shstats));
+
+ if (i == 0)
+ pgStatLocal.shmem->io.stats.stat_reset_timestamp = ts;
+ }
+
+ // Release LWLocks
+ for (int i = 0; i < STATS_SHARED_NUM_LWLOCK; i++)
+ LWLockRelease(locks[i]);
+
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ {
+ LWLock *bktype_lock = &stats_io->locks[i];
+ LWLockRelease(bktype_lock);
+ }
+}
/* ------------------------------------------------------------
* Fetching of stats
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 1fb8b31863..0d6f375238 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1676,6 +1676,16 @@ pg_stat_reset(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+/*
+ * Reset all shared cluster-wide counters
+ */
+Datum
+pg_stat_reset_shared_all(PG_FUNCTION_ARGS)
+{
+ pgstat_reset_shared_all();
+ PG_RETURN_VOID();
+}
+
/*
* Reset some shared cluster-wide counters
*
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 091f7e343c..afda903f24 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5883,6 +5883,10 @@
descr => 'statistics: reset collected statistics shared across the cluster',
proname => 'pg_stat_reset_shared', provolatile => 'v', prorettype => 'void',
proargtypes => 'text', prosrc => 'pg_stat_reset_shared' },
+{ oid => '8000',
+ descr => 'statistics: reset collected statistics shared across the cluster',
+ proname => 'pg_stat_reset_shared', provolatile => 'v', prorettype => 'void',
+ proargtypes => '', prosrc => 'pg_stat_reset_shared_all' },
{ oid => '3776',
descr => 'statistics: reset collected statistics for a single table or index in the current database or shared across all databases in the cluster',
proname => 'pg_stat_reset_single_table_counters', provolatile => 'v',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index f95d8db0c4..2b238bee78 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -477,6 +477,7 @@ extern void pgstat_force_next_flush(void);
extern void pgstat_reset_counters(void);
extern void pgstat_reset(PgStat_Kind kind, Oid dboid, Oid objoid);
extern void pgstat_reset_of_kind(PgStat_Kind kind);
+extern void pgstat_reset_shared_all(void);
/* stats accessors */
extern void pgstat_clear_snapshot(void);
base-commit: 2c7c6c417fe655ab3fd4ca7f68ec22c913a2fe80
--
2.39.2
