On 2023-11-15 09:47, Michael Paquier wrote:
On Tue, Nov 14, 2023 at 10:02:32PM +0900, torikoshia wrote:
Attached patch.
You have forgotten to update the errhint at the end of
pg_stat_reset_shared(), where "slru" needs to be listed :)
Oops, attached v2 patch.
BTW currently the documentation explains all the arguments of
pg_stat_reset_shared() in one line and I feel it's a bit hard to read.
Attached patch uses <itemizedlist>.
Yes, this one is a good idea because each target works on a different
system view so it becomes easier to understand what a target affects,
so I've applied this bit, without the slru addition.
Thanks!
--
Michael
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
From 9283d25cb96c9c4253d7f39c464b61576bd52a52 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Wed, 15 Nov 2023 11:47:13 +0900
Subject: [PATCH v2] Add ability to reset pg_stat_slru in
pg_stat_reset_shared()
Currently, pg_stat_reset_shared() can not reset pg_stat_slru
nevertheless this is one of shared statistics view.
This patch adds a new argment 'slru' to reset all the rows
in pg_stat_slru.
Considering pg_stat_reset_slru() was introduced in
28cac71bd368(PostgreSQL 13) and it could impact existing
applications and it can not only reset all the entry but
can specify which entry to reset, this function is left
as it is.
---
doc/src/sgml/monitoring.sgml | 6 +++++
src/backend/utils/adt/pgstatfuncs.c | 5 +++-
src/test/regress/expected/stats.out | 37 ++++++++++++++++++++++++++++-
src/test/regress/sql/stats.sql | 7 ++++++
4 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 1e9913c8bc..42509042ad 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4749,6 +4749,12 @@ description | Waiting for a newly initialized WAL file to reach durable storage
the <structname>pg_stat_recovery_prefetch</structname> view.
</para>
</listitem>
+ <listitem>
+ <para>
+ <literal>slru</literal>: Reset all the counters shown in the
+ <structname>pg_stat_slru</structname> view.
+ </para>
+ </listitem>
<listitem>
<para>
<literal>wal</literal>: Reset all the counters shown in the
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 3a9f9bc4fe..0cea320c00 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1695,6 +1695,7 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
pgstat_reset_of_kind(PGSTAT_KIND_CHECKPOINTER);
pgstat_reset_of_kind(PGSTAT_KIND_IO);
XLogPrefetchResetStats();
+ pgstat_reset_of_kind(PGSTAT_KIND_SLRU);
pgstat_reset_of_kind(PGSTAT_KIND_WAL);
PG_RETURN_VOID();
@@ -1712,13 +1713,15 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
pgstat_reset_of_kind(PGSTAT_KIND_IO);
else if (strcmp(target, "recovery_prefetch") == 0)
XLogPrefetchResetStats();
+ else if (strcmp(target, "slru") == 0)
+ pgstat_reset_of_kind(PGSTAT_KIND_SLRU);
else if (strcmp(target, "wal") == 0)
pgstat_reset_of_kind(PGSTAT_KIND_WAL);
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("unrecognized reset target: \"%s\"", target),
- errhint("Target must be \"archiver\", \"bgwriter\", \"checkpointer\", \"io\", \"recovery_prefetch\", or \"wal\".")));
+ errhint("Target must be \"archiver\", \"bgwriter\", \"checkpointer\", \"io\", \"recovery_prefetch\", \"slru\", or \"wal\".")));
PG_RETURN_VOID();
}
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 6ed3584a76..103ea453df 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -960,6 +960,28 @@ SELECT stats_reset > :'recovery_prefetch_reset_ts'::timestamptz FROM pg_stat_rec
(1 row)
SELECT stats_reset AS recovery_prefetch_reset_ts FROM pg_stat_recovery_prefetch \gset
+-- Test that reset_shared with slru specified as the stats type works
+SELECT MAX(stats_reset) AS slru_reset_ts FROM pg_stat_slru \gset
+SELECT pg_stat_reset_shared('slru');
+ pg_stat_reset_shared
+----------------------
+
+(1 row)
+
+SELECT stats_reset > :'slru_reset_ts'::timestamptz FROM pg_stat_slru;
+ ?column?
+----------
+ t
+ t
+ t
+ t
+ t
+ t
+ t
+ t
+(8 rows)
+
+SELECT MAX(stats_reset) AS slru_reset_ts FROM pg_stat_slru \gset
-- Test that reset_shared with wal specified as the stats type works
SELECT stats_reset AS wal_reset_ts FROM pg_stat_wal \gset
SELECT pg_stat_reset_shared('wal');
@@ -1007,6 +1029,19 @@ SELECT stats_reset > :'recovery_prefetch_reset_ts'::timestamptz FROM pg_stat_rec
t
(1 row)
+SELECT stats_reset > :'slru_reset_ts'::timestamptz FROM pg_stat_slru;
+ ?column?
+----------
+ t
+ t
+ t
+ t
+ t
+ t
+ t
+ t
+(8 rows)
+
SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
?column?
----------
@@ -1016,7 +1051,7 @@ SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
-- Test error case for reset_shared with unknown stats type
SELECT pg_stat_reset_shared('unknown');
ERROR: unrecognized reset target: "unknown"
-HINT: Target must be "archiver", "bgwriter", "checkpointer", "io", "recovery_prefetch", or "wal".
+HINT: Target must be "archiver", "bgwriter", "checkpointer", "io", "recovery_prefetch", "slru", or "wal".
-- Test that reset works for pg_stat_database
-- Since pg_stat_database stats_reset starts out as NULL, reset it once first so we have something to compare it to
SELECT pg_stat_reset();
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index fd8afeeae5..add461e5a0 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -482,6 +482,12 @@ SELECT pg_stat_reset_shared('recovery_prefetch');
SELECT stats_reset > :'recovery_prefetch_reset_ts'::timestamptz FROM pg_stat_recovery_prefetch;
SELECT stats_reset AS recovery_prefetch_reset_ts FROM pg_stat_recovery_prefetch \gset
+-- Test that reset_shared with slru specified as the stats type works
+SELECT MAX(stats_reset) AS slru_reset_ts FROM pg_stat_slru \gset
+SELECT pg_stat_reset_shared('slru');
+SELECT stats_reset > :'slru_reset_ts'::timestamptz FROM pg_stat_slru;
+SELECT MAX(stats_reset) AS slru_reset_ts FROM pg_stat_slru \gset
+
-- Test that reset_shared with wal specified as the stats type works
SELECT stats_reset AS wal_reset_ts FROM pg_stat_wal \gset
SELECT pg_stat_reset_shared('wal');
@@ -495,6 +501,7 @@ SELECT stats_reset > :'archiver_reset_ts'::timestamptz FROM pg_stat_archiver;
SELECT stats_reset > :'bgwriter_reset_ts'::timestamptz FROM pg_stat_bgwriter;
SELECT stats_reset > :'checkpointer_reset_ts'::timestamptz FROM pg_stat_checkpointer;
SELECT stats_reset > :'recovery_prefetch_reset_ts'::timestamptz FROM pg_stat_recovery_prefetch;
+SELECT stats_reset > :'slru_reset_ts'::timestamptz FROM pg_stat_slru;
SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
-- Test error case for reset_shared with unknown stats type
base-commit: f26c2368dcaef7519d8400ca958c4b5742cdbd46
--
2.39.2