Re: pgstat: allow a stats kind to use its own dedicated dsa/dshash

Hi,

On Wed, Jul 15, 2026 at 12:16:26PM -0500, Sami Imseih wrote:
> The attached proposal implements a new stats kind option, own_hash, which
> when set to true allocates an independent dsa/dshash for that stats kind.

Thanks for the patch!

The dedicated hash idea makes sense to me.

A few comments:

=== 1

- shhashent = dshash_find_or_insert(pgStatLocal.shared_hash, &key, &shfound);
+ shhashent = dshash_find_or_insert(hash, &key, &shfound);

With the patch, an extension can set a size limit on its dedicated DSA, making
an allocation failure much easier to reach. Such a failure in dshash_find_or_insert()
would raise ERROR, leaving the backend local cache entry with a NULL shared_entry.

A later pgstat_gc_entry_refs() call could then dereference the NULL
entry_ref->shared_entry pointer.

I wonder if this should use dshash_find_or_insert_extended(..., DSHASH_INSERT_NO_OOM)
and release the local entry if it returns NULL?

=== 2

@@ -2080,12 +2090,12 @@ pgstat_read_statsfile(void)
* putting all stats into checkpointer's
* pgStatEntryRefHash would be wasted effort and memory.
*/
- p = dshash_find_or_insert(pgStatLocal.shared_hash, &key, &found);
+ p = dshash_find_or_insert(pgstat_get_hash_for_kind(key.kind), &key, &found);

If the dedicated DSA cannot accommodate the persisted entries, dshash_find_or_insert()
raises ERROR and startup can fail.

I wonder if this case should discard the statistics that no longer fit rather
than prevent the server from starting?

=== 3

+ /* Add per-kind DSA space for own_hash kinds */
+ for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
+ {
+ const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);

I wonder if it wouldn't make more sense for own_hash to create a dedicated dshash
in the existing pgstat DSA, with a separate option for a dedicated DSA?

That would retain independent iteration and hash partition locks while avoiding
an additional shared memory allocation and per backend DSA state when independent
memory accounting is not needed.

=== 4

+ * Returns the DSA area for a given kind. Kinds with own_hash set have
+ * a dedicated DSA; others use the shared DSA.
+ */
+static inline dsa_area *
+pgstat_get_dsa_for_kind(PgStat_Kind kind)
+{
+ if (pgStatLocal.kind_dsa[kind] != NULL)
+ return pgStatLocal.kind_dsa[kind];
+

Current master provides dshash_get_dsa_area() since 762e329e83f, so this one
looks now redundant.

=== 5

pgstat_reset_matching_entries(bool (*do_reset) (PgStatShared_HashEntry *, Datum),
@@ -1173,26 +1282,29 @@ pgstat_reset_matching_entries(bool (*do_reset) (PgStatShared_HashEntry *, Datum)
PgStatShared_HashEntry *p;

/* dshash entry is not modified, take shared lock */
- dshash_seq_init(&hstat, pgStatLocal.shared_hash, false);
- while ((p = dshash_seq_next(&hstat)) != NULL)
+ for (int h = 0; h < pgStatLocal.num_hashes; h++)
{
- PgStatShared_Common *header;
+ dshash_seq_init(&hstat, pgStatLocal.all_hashes[h], false);
+ while ((p = dshash_seq_next(&hstat)) != NULL)
+ {

pgstat_reset_entries_of_kind() uses this routine, so resetting one kind still
scans every hash, including unrelated dedicated hashes.

Could pgstat_reset_entries_of_kind() scan only the hash returned by
pgstat_get_hash_for_kind(kind)? The kind filter would still be needed when
that returns the shared hash.

=== 6

- /* Register custom statistics kind */
- pgstat_register_kind(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, &custom_stats);
+ /* Must be loaded via shared_preload_libraries */
+ if (!process_shared_preload_libraries_in_progress)
+ return;
+

This restores the behavior removed by 5045d9ff3b5. The SQL functions remain
callable with the kind unregistered.

I think the branch should be removed so pgstat_register_kind() reports the
intended prerequisite error.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com