On Thu, Nov 09, 2023 at 10:10:39AM +0900, torikoshia wrote:
I'll attach the patch.
Attached.
On Mon, Nov 6, 2023 at 5:30 PM Bharath Rupireddy
3. I think the new reset all stats function must also consider
resetting all SLRU stats, no?
/* stats for fixed-numbered objects */
PGSTAT_KIND_ARCHIVER,
PGSTAT_KIND_BGWRITER,
PGSTAT_KIND_CHECKPOINTER,
PGSTAT_KIND_IO,
PGSTAT_KIND_SLRU,
PGSTAT_KIND_WAL,
PGSTAT_KIND_SLRU cannot be reset by pg_stat_reset_shared(), so I feel
uncomfortable to delete it all together.
It might be better after pg_stat_reset_shared() has been modified to
take 'slru' as an argument, though.
On Wed, Nov 8, 2023 at 1:13 PM Andres Freund <and...@anarazel.de> wrote:
It's not like oids are a precious resource. It's a more confusing API
to have
to have to specify a NULL as an argument than not having to do so. If
we
really want to avoid a separate oid, a more sensible path would be to
add a
default argument to pg_stat_reset_slru() (by doing a CREATE OR REPLACE
in
system_functions.sql).
Currently proisstrict is true and pg_stat_reset_shared() returns null
without doing any work.
I thought it would be better to reset statistics even when null is
specified so that users are not confused with the behavior of
pg_stat_reset_slru().
Attached patch added pg_stat_reset_shared() in system_functions.sql
mainly for this reason.
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
From 931bdac6e70b681e3416bbf464cbc6f42f971c6c Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Thu, 9 Nov 2023 13:41:51 +0900
Subject: [PATCH v1] 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 or NULL is specified.
---
doc/src/sgml/monitoring.sgml | 4 +++-
src/backend/catalog/system_functions.sql | 7 +++++++
src/backend/utils/adt/pgstatfuncs.c | 19 +++++++++++++++++--
src/include/catalog/pg_proc.dat | 5 +++--
src/test/regress/expected/stats.out | 14 +++++++-------
src/test/regress/sql/stats.sql | 14 +++++++-------
6 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index e068f7e247..1cddc7d238 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 the argument is NULL or not specified, all counters shown in all
+ of these views are reset.
</para>
<para>
This function is restricted to superusers by default, but other users
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 35d738d576..8079f1cd7f 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -621,6 +621,13 @@ LANGUAGE internal
STRICT IMMUTABLE PARALLEL SAFE
AS 'unicode_is_normalized';
+CREATE OR REPLACE FUNCTION
+ pg_stat_reset_shared(target text DEFAULT NULL)
+RETURNS void
+LANGUAGE INTERNAL
+CALLED ON NULL INPUT VOLATILE PARALLEL SAFE
+AS 'pg_stat_reset_shared';
+
--
-- The default permissions for functions mean that anyone can execute them.
-- A number of functions shouldn't be executable by just anyone, but rather
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 1fb8b31863..e3d78b9665 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1677,7 +1677,7 @@ pg_stat_reset(PG_FUNCTION_ARGS)
}
/*
- * Reset some shared cluster-wide counters
+ * Reset shared cluster-wide counters
*
* When adding a new reset target, ideally the name should match that in
* pgstat_kind_infos, if relevant.
@@ -1685,7 +1685,22 @@ pg_stat_reset(PG_FUNCTION_ARGS)
Datum
pg_stat_reset_shared(PG_FUNCTION_ARGS)
{
- char *target = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ char *target = NULL;
+
+ if (PG_ARGISNULL(0))
+ {
+ /* Reset all the statistics which can be specified by the argument */
+ pgstat_reset_of_kind(PGSTAT_KIND_ARCHIVER);
+ pgstat_reset_of_kind(PGSTAT_KIND_BGWRITER);
+ pgstat_reset_of_kind(PGSTAT_KIND_CHECKPOINTER);
+ pgstat_reset_of_kind(PGSTAT_KIND_IO);
+ pgstat_reset_of_kind(PGSTAT_KIND_WAL);
+ XLogPrefetchResetStats();
+
+ PG_RETURN_VOID();
+ }
+
+ target = text_to_cstring(PG_GETARG_TEXT_PP(0));
if (strcmp(target, "archiver") == 0)
pgstat_reset_of_kind(PGSTAT_KIND_ARCHIVER);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index f14aed422a..d36144fbc6 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5879,10 +5879,11 @@
descr => 'statistics: reset collected statistics for current database',
proname => 'pg_stat_reset', proisstrict => 'f', provolatile => 'v',
prorettype => 'void', proargtypes => '', prosrc => 'pg_stat_reset' },
+
{ oid => '3775',
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' },
+ proname => 'pg_stat_reset_shared', proisstrict => 'f', provolatile => 'v',
+ prorettype => 'void', proargtypes => 'text', prosrc => 'pg_stat_reset_shared' },
{ 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/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 494cef07d3..dfcb451543 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -975,38 +975,38 @@ SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
(1 row)
SELECT stats_reset AS wal_reset_ts FROM pg_stat_wal \gset
--- Test that reset_shared with no specified stats type doesn't reset anything
-SELECT pg_stat_reset_shared(NULL);
+-- Test that all shared stats are reset when no argument provided to reset function
+SELECT pg_stat_reset_shared();
pg_stat_reset_shared
----------------------
(1 row)
-SELECT stats_reset = :'archiver_reset_ts'::timestamptz FROM pg_stat_archiver;
+SELECT stats_reset > :'archiver_reset_ts'::timestamptz FROM pg_stat_archiver;
?column?
----------
t
(1 row)
-SELECT stats_reset = :'bgwriter_reset_ts'::timestamptz FROM pg_stat_bgwriter;
+SELECT stats_reset > :'bgwriter_reset_ts'::timestamptz FROM pg_stat_bgwriter;
?column?
----------
t
(1 row)
-SELECT stats_reset = :'checkpointer_reset_ts'::timestamptz FROM pg_stat_checkpointer;
+SELECT stats_reset > :'checkpointer_reset_ts'::timestamptz FROM pg_stat_checkpointer;
?column?
----------
t
(1 row)
-SELECT stats_reset = :'recovery_prefetch_reset_ts'::timestamptz FROM pg_stat_recovery_prefetch;
+SELECT stats_reset > :'recovery_prefetch_reset_ts'::timestamptz FROM pg_stat_recovery_prefetch;
?column?
----------
t
(1 row)
-SELECT stats_reset = :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
+SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
?column?
----------
t
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 7ae8b8a276..3d6cbbafb5 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -488,13 +488,13 @@ SELECT pg_stat_reset_shared('wal');
SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
SELECT stats_reset AS wal_reset_ts FROM pg_stat_wal \gset
--- Test that reset_shared with no specified stats type doesn't reset anything
-SELECT pg_stat_reset_shared(NULL);
-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 = :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
+-- Test that all shared stats are reset when no argument provided to reset function
+SELECT pg_stat_reset_shared();
+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 > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
-- Test error case for reset_shared with unknown stats type
SELECT pg_stat_reset_shared('unknown');
base-commit: cd694f60dc975e9fe41e8643ca6f0629283d102e
--
2.39.2
