On Wed, Nov 8, 2023 at 9:43 AM Andres Freund <and...@anarazel.de> wrote:
>
> > 2.
> > +{ 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' },
> >
> > Why a new function consuming the oid? Why can't we just do the trick
> > of proisstrict => 'f' and if (PG_ARGISNULL(0)) { reset all stats} else
> > {reset specified stats kind} like the pg_stat_reset_slru()?
>
> 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).
+1. Attached the patch.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 8b70989fcf88e92cd005852dd75a770324f3de3e Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com>
Date: Wed, 8 Nov 2023 08:33:19 +0000
Subject: [PATCH v1] Add no argument support for pg_stat_reset_slru
Presently, one needs to specify pg_stat_reset_slru input argument
as NULL to reset all SLRU stats which looks a bit odd confusing to
the user. This commit changes the API by adding a DEFAULT NULL to
input parameter in system_functions.sql allowing users to not pass
anything as input to reset all SLRU stats.
---
doc/src/sgml/monitoring.sgml | 6 +++---
src/backend/catalog/system_functions.sql | 7 +++++++
src/include/catalog/pg_proc.dat | 3 ++-
src/test/regress/expected/stats.out | 2 +-
src/test/regress/sql/stats.sql | 2 +-
5 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a0257fea0c..87873a3c29 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4844,9 +4844,9 @@ description | Waiting for a newly initialized WAL file to
reach durable storage
</para>
<para>
Resets statistics to zero for a single SLRU cache, or for all SLRUs in
- the cluster. If the argument is NULL, all counters shown in
- the <structname>pg_stat_slru</structname> view for all SLRU caches are
- reset. The argument can be one of
+ the cluster. If the argument is NULL or not specified, all counters
+ shown in the <structname>pg_stat_slru</structname> view for all SLRU
+ caches are reset. The argument can be one of
<literal>CommitTs</literal>,
<literal>MultiXactMember</literal>,
<literal>MultiXactOffset</literal>,
diff --git a/src/backend/catalog/system_functions.sql
b/src/backend/catalog/system_functions.sql
index 35d738d576..2546912436 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_slru(target text DEFAULT NULL)
+RETURNS void
+LANGUAGE INTERNAL
+CALLED ON NULL INPUT VOLATILE PARALLEL SAFE
+AS 'pg_stat_reset_slru';
+
--
-- 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/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 4af16a0f81..7999da77f4 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5896,7 +5896,8 @@
{ oid => '2307',
descr => 'statistics: reset collected statistics for a single SLRU',
proname => 'pg_stat_reset_slru', proisstrict => 'f', provolatile => 'v',
- prorettype => 'void', proargtypes => 'text', prosrc => 'pg_stat_reset_slru'
},
+ prorettype => 'void', proargtypes => 'text', proargnames => '{target}',
+ prosrc => 'pg_stat_reset_slru' },
{ oid => '6170',
descr => 'statistics: reset collected statistics for a single replication
slot',
proname => 'pg_stat_reset_replication_slot', proisstrict => 'f',
diff --git a/src/test/regress/expected/stats.out
b/src/test/regress/expected/stats.out
index 494cef07d3..3355adb956 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -882,7 +882,7 @@ SELECT stats_reset >
:'slru_commit_ts_reset_ts'::timestamptz FROM pg_stat_slru W
SELECT stats_reset AS slru_commit_ts_reset_ts FROM pg_stat_slru WHERE name =
'CommitTs' \gset
-- Test that multiple SLRUs are reset when no specific SLRU provided to reset
function
-SELECT pg_stat_reset_slru(NULL);
+SELECT pg_stat_reset_slru();
pg_stat_reset_slru
--------------------
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 7ae8b8a276..f8fc38eea7 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -454,7 +454,7 @@ SELECT stats_reset >
:'slru_commit_ts_reset_ts'::timestamptz FROM pg_stat_slru W
SELECT stats_reset AS slru_commit_ts_reset_ts FROM pg_stat_slru WHERE name =
'CommitTs' \gset
-- Test that multiple SLRUs are reset when no specific SLRU provided to reset
function
-SELECT pg_stat_reset_slru(NULL);
+SELECT pg_stat_reset_slru();
SELECT stats_reset > :'slru_commit_ts_reset_ts'::timestamptz FROM pg_stat_slru
WHERE name = 'CommitTs';
SELECT stats_reset > :'slru_notify_reset_ts'::timestamptz FROM pg_stat_slru
WHERE name = 'Notify';
--
2.34.1
