From: torikoshia <torikoshia@oss.nttdata.com>
To: Andres Freund <andres@anarazel.de>
To: bharath.rupireddyforpostgres@gmail.com
To: michael@paquier.xyz
Cc: boekewurm+postgres@gmail.com, Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Add new option 'all' to pg_stat_reset_shared()
Date: Thu, 09 Nov 2023 13:50:34 +0900
Message-ID: <37bce6feca33e0c0441cd33f9eaa51d6@oss.nttdata.com> (raw)
In-Reply-To: <20231109012952.fft2i356ofljugt6@awork3.anarazel.de>
References: <ZUGFttiY_DgwUk2s@paquier.xyz>
<CALj2ACXQTKMYL-aYKM=XObQ27wyD3V2yRGeF1z=jJN7vptgNgg@mail.gmail.com>
<20231104014905.oiropxau5can5fdu@awork3.anarazel.de>
<e784c0d54c849e86d69138883d12138f@oss.nttdata.com>
<CALj2ACVjpHfkCUWSMfpX-oyuqDCenbbduxNfQWPUBXZ0TChMrg@mail.gmail.com>
<20231108041331.mmxtbr3cmufy4f3v@awork3.anarazel.de>
<CALj2ACW4Fqc_m+OaavrOMEivZ5aBa24pVKvoXRTmuFECsNBfAg@mail.gmail.com>
<ZUwguPB1rTXBK5CT@paquier.xyz>
<62d8244b05557a91e25914c02607b493@oss.nttdata.com>
<ZUw0_tA7g0dkTow8@paquier.xyz>
<20231109012952.fft2i356ofljugt6@awork3.anarazel.de>
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 <andres@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
Attachments:v1-0001-Add-ability-to-reset-all-statistics-to-pg_stat_re.patch (text/x-diff, 8.1K) download
From 931bdac6e70b681e3416bbf464cbc6f42f971c6c Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikoshia@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.sgmlindex 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.sqlindex 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.cindex 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.datindex 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.outindex 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.sqlindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: torikoshia@oss.nttdata.com, andres@anarazel.de, bharath.rupireddyforpostgres@gmail.com, michael@paquier.xyz, horikyota.ntt@gmail.com
Subject: Re: Add new option 'all' to pg_stat_reset_shared()
In-Reply-To: <37bce6feca33e0c0441cd33f9eaa51d6@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox