On Wed, Sep 24, 2025 at 11:37:25AM +0900, Keisuke Kuroda wrote: > Regarding the option name track_counts in PgStat_KindInfo. > In my personal opinion, I was just wondering that it shares > the same name as the GUC track_counts(pgstat_track_counts in the source code). > If we want to make it clearer, renaming it to track_entry_counts > could be one option.
Yes, that's a good point and I have missed the GUC part. What you are suggesting is cleaner overall with the flag added to the pgstats kind info. > (That said, I feel the GUC track_counts option has a mismatch > between its role and its name...) I doubt that we will rename this GUC at any point in the future. Updated patch attached with the new name, as you are suggesting. -- Michael
From 0b38ea6e3ef9cd30482983c0fda102472967b7dd Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 25 Sep 2025 09:37:23 +0900
Subject: [PATCH v2 1/2] Add support for entry counting in pgstats
Stats kinds can set an option call track_counts, that will make pgstats
track the number of entries that exist in the shared hashtable.
As there is only one code path where a new entry is added, and one code
path where entries are removed, the count tracking is rather
straight-forward. Reads of these counters are optimistic, and may
change across two calls.
A use case of this facility is PGSS, where we need to be able to cap the
number of entries that would be stored in the shared hashtable, based on
its "max" GUC.
---
src/include/utils/pgstat_internal.h | 26 +++++++++++++
src/backend/utils/activity/pgstat.c | 6 +++
src/backend/utils/activity/pgstat_shmem.c | 47 +++++++++++++++++------
3 files changed, 67 insertions(+), 12 deletions(-)
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index bf75ebcef31c..6daf20d0fefa 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -231,6 +231,12 @@ typedef struct PgStat_KindInfo
/* Should stats be written to the on-disk stats file? */
bool write_to_file:1;
+ /*
+ * Should the number of entries be tracked? For variable-numbered stats,
+ * to update its PgStat_ShmemControl.entry_counts.
+ */
+ bool track_entry_counts:1;
+
/*
* The size of an entry in the shared stats hash table (pointed to by
* PgStatShared_HashEntry->body). For fixed-numbered statistics, this is
@@ -500,6 +506,15 @@ typedef struct PgStat_ShmemControl
*/
pg_atomic_uint64 gc_request_count;
+ /*
+ * Counters for the number of entries associated to a single stats kind
+ * that uses variable-numbered objects stored in the shared hash table.
+ * These counters can be enabled on a per-kind basis, when track_counts is
+ * set. This counter is incremented each time a new entry is created (not
+ * when reused), and each time an entry is dropped.
+ */
+ pg_atomic_uint64 entry_counts[PGSTAT_KIND_MAX];
+
/*
* Stats data for fixed-numbered objects.
*/
@@ -925,6 +940,17 @@ pgstat_get_entry_data(PgStat_Kind kind, PgStatShared_Common *entry)
return ((char *) (entry)) + off;
}
+/*
+ * Returns the number of entries counted for a stats kind.
+ */
+static inline uint64
+pgstat_get_entry_count(PgStat_Kind kind)
+{
+ Assert(pgstat_get_kind_info(kind)->track_entry_counts);
+
+ return pg_atomic_read_u64(&pgStatLocal.shmem->entry_counts[kind - 1]);
+}
+
/*
* Returns a pointer to the shared memory area of custom stats for
* fixed-numbered statistics.
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 73c2ced3f4e2..6a53259b8d77 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1487,6 +1487,12 @@ pgstat_register_kind(PgStat_Kind kind, const PgStat_KindInfo *kind_info)
ereport(ERROR,
(errmsg("custom cumulative statistics property is invalid"),
errhint("Custom cumulative statistics require a shared memory size for fixed-numbered objects.")));
+ if (kind_info->track_entry_counts)
+ {
+ ereport(ERROR,
+ (errmsg("custom cumulative statistics property is invalid"),
+ errhint("Custom cumulative statistics cannot use counter tracking for fixed-numbered objects.")));
+ }
}
/*
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index ca36fd247f64..066027c387c4 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -210,27 +210,35 @@ StatsShmemInit(void)
pg_atomic_init_u64(&ctl->gc_request_count, 1);
- /* initialize fixed-numbered stats */
+ /* Do the per-kind initialization */
for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
{
const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
char *ptr;
- if (!kind_info || !kind_info->fixed_amount)
+ if (!kind_info)
continue;
- if (pgstat_is_kind_builtin(kind))
- ptr = ((char *) ctl) + kind_info->shared_ctl_off;
- else
+ /* initialize counter tracking */
+ if (kind_info->track_entry_counts)
+ pg_atomic_init_u64(&ctl->entry_counts[kind - 1], 0);
+
+ /* initialize fixed-numbered stats */
+ if (kind_info->fixed_amount)
{
- int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
+ if (pgstat_is_kind_builtin(kind))
+ ptr = ((char *) ctl) + kind_info->shared_ctl_off;
+ else
+ {
+ int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
- Assert(kind_info->shared_size != 0);
- ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
- ptr = ctl->custom_data[idx];
+ Assert(kind_info->shared_size != 0);
+ ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
+ ptr = ctl->custom_data[idx];
+ }
+
+ kind_info->init_shmem_cb(ptr);
}
-
- kind_info->init_shmem_cb(ptr);
}
}
else
@@ -303,6 +311,7 @@ pgstat_init_entry(PgStat_Kind kind,
/* Create new stats entry. */
dsa_pointer chunk;
PgStatShared_Common *shheader;
+ const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
/*
* Initialize refcount to 1, marking it as valid / not dropped. The entry
@@ -319,7 +328,7 @@ pgstat_init_entry(PgStat_Kind kind,
shhashent->dropped = false;
chunk = dsa_allocate_extended(pgStatLocal.dsa,
- pgstat_get_kind_info(kind)->shared_size,
+ kind_info->shared_size,
DSA_ALLOC_ZERO | DSA_ALLOC_NO_OOM);
if (chunk == InvalidDsaPointer)
return NULL;
@@ -330,6 +339,10 @@ pgstat_init_entry(PgStat_Kind kind,
/* Link the new entry from the hash entry. */
shhashent->body = chunk;
+ /* Increment entry count, if required. */
+ if (kind_info->track_entry_counts)
+ pg_atomic_fetch_add_u64(&pgStatLocal.shmem->entry_counts[kind - 1], 1);
+
LWLockInitialize(&shheader->lock, LWTRANCHE_PGSTATS_DATA);
return shheader;
@@ -907,7 +920,17 @@ pgstat_drop_entry_internal(PgStatShared_HashEntry *shent,
/* release refcount marking entry as not dropped */
if (pg_atomic_sub_fetch_u32(&shent->refcount, 1) == 0)
{
+ PgStat_Kind kind = shent->key.kind;
+
pgstat_free_entry(shent, hstat);
+
+ /*
+ * Entry has been dropped with refcount at 0, hence decrement the
+ * entry counter.
+ */
+ if (pgstat_get_kind_info(kind)->track_entry_counts)
+ pg_atomic_sub_fetch_u64(&pgStatLocal.shmem->entry_counts[kind - 1], 1);
+
return true;
}
else
--
2.51.0
From 538e0bd0a896a914bfb157d8442b24c08b0345fc Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 25 Sep 2025 09:37:58 +0900
Subject: [PATCH v2 2/2] injection_points: Add entry counting
This serves as coverage for the track_counts, as other in-core stats
kinds do not need to cap their maximum.
---
.../injection_points/injection_points--1.0.sql | 10 ++++++++++
src/test/modules/injection_points/injection_stats.c | 12 ++++++++++++
src/test/modules/injection_points/t/001_stats.pl | 12 ++++++++++++
3 files changed, 34 insertions(+)
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 5f5657b2043c..a7b61fbdfe6f 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -99,6 +99,16 @@ RETURNS bigint
AS 'MODULE_PATHNAME', 'injection_points_stats_numcalls'
LANGUAGE C STRICT;
+--
+-- injection_points_stats_count()
+--
+-- Return the number of entries stored in the pgstats hash table.
+--
+CREATE FUNCTION injection_points_stats_count()
+RETURNS bigint
+AS 'MODULE_PATHNAME', 'injection_points_stats_count'
+LANGUAGE C STRICT;
+
--
-- injection_points_stats_drop()
--
diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c
index ca8df4ad217a..ae6951372308 100644
--- a/src/test/modules/injection_points/injection_stats.c
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -49,6 +49,7 @@ static const PgStat_KindInfo injection_stats = {
.shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats),
.pending_size = sizeof(PgStat_StatInjEntry),
.flush_pending_cb = injection_stats_flush_cb,
+ .track_entry_counts = true,
};
/*
@@ -196,6 +197,17 @@ injection_points_stats_numcalls(PG_FUNCTION_ARGS)
PG_RETURN_INT64(entry->numcalls);
}
+/*
+ * SQL function returning the number of entries allocated for injection
+ * points in the shared hashtable of pgstats.
+ */
+PG_FUNCTION_INFO_V1(injection_points_stats_count);
+Datum
+injection_points_stats_count(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_INT64(pgstat_get_entry_count(PGSTAT_KIND_INJECTION));
+}
+
/* Only used by injection_points_stats_drop() */
static bool
match_inj_entries(PgStatShared_HashEntry *entry, Datum match_data)
diff --git a/src/test/modules/injection_points/t/001_stats.pl b/src/test/modules/injection_points/t/001_stats.pl
index 25de5fc46fe4..47ab58d0e9b8 100644
--- a/src/test/modules/injection_points/t/001_stats.pl
+++ b/src/test/modules/injection_points/t/001_stats.pl
@@ -36,6 +36,9 @@ $node->safe_psql('postgres', "SELECT injection_points_run('stats-notice');");
my $numcalls = $node->safe_psql('postgres',
"SELECT injection_points_stats_numcalls('stats-notice');");
is($numcalls, '2', 'number of stats calls');
+my $entrycount =
+ $node->safe_psql('postgres', "SELECT injection_points_stats_count();");
+is($entrycount, '1', 'number of entries');
my $fixedstats = $node->safe_psql('postgres',
"SELECT * FROM injection_points_stats_fixed();");
is($fixedstats, '1|0|2|0|0', 'fixed stats after some calls');
@@ -55,6 +58,9 @@ $node->restart;
$numcalls = $node->safe_psql('postgres',
"SELECT injection_points_stats_numcalls('stats-notice');");
is($numcalls, '3', 'number of stats after clean restart');
+$entrycount =
+ $node->safe_psql('postgres', "SELECT injection_points_stats_count();");
+is($entrycount, '1', 'number of entries after clean restart');
$fixedstats = $node->safe_psql('postgres',
"SELECT * FROM injection_points_stats_fixed();");
is($fixedstats, '1|0|2|1|1', 'fixed stats after clean restart');
@@ -65,6 +71,9 @@ $node->start;
$numcalls = $node->safe_psql('postgres',
"SELECT injection_points_stats_numcalls('stats-notice');");
is($numcalls, '', 'number of stats after crash');
+$entrycount =
+ $node->safe_psql('postgres', "SELECT injection_points_stats_count();");
+is($entrycount, '0', 'number of entries after crash');
$fixedstats = $node->safe_psql('postgres',
"SELECT * FROM injection_points_stats_fixed();");
is($fixedstats, '0|0|0|0|0', 'fixed stats after crash');
@@ -81,6 +90,9 @@ $node->safe_psql('postgres', "SELECT injection_points_stats_drop();");
$numcalls = $node->safe_psql('postgres',
"SELECT injection_points_stats_numcalls('stats-notice');");
is($numcalls, '', 'no stats after drop via SQL function');
+$entrycount =
+ $node->safe_psql('postgres', "SELECT injection_points_stats_count();");
+is($entrycount, '0', 'number of entries after drop via SQL function');
# Stop the server, disable the module, then restart. The server
# should be able to come up.
--
2.51.0
signature.asc
Description: PGP signature
