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

From: Sami Imseih <samimseih(dot)pg(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgstat: allow a stats kind to use its own dedicated dsa/dshash
Date: 2026-10-07 21:55:21
Message-ID: CAN12+Y+VJ7B_GBr5dADJG+mCUE7=zWXe3J8y90iOR_Lr4JGhiw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> There is a lot of back-and-forth with dshash_get_dsa_area() as a
> matter to replace pgStatLocal.dsa. Hmm, dshash_get_dsa_area() only
> retrieves a dshash_table->area, but wouldn't it be simpler to replace
> the single pgStatLocal.dsa with an area based on the kind ID?

Done in v4. pgStatLocal.dsa is replaced by pgStatLocal.kind_dsa[],
so pgStatLocal.kind_hash[kind] and pgStatLocal.kind_dsa[kind] can be
used directly instead of passing a dsa_area around.

> Or perhaps introduce an inline pgstat_get_dsaarea_for_kind() with an
> Assert(), something that v2 did not do either.

I used the array lookup directly instead. This also addresses your later
point about the value of an assert-only helper, and keeps the DSA lookup
consistent with the hash lookup after removing pgstat_get_hash_for_kind().

> - pgstat_get_kind_info(shent->key.kind)->name,
> + kind_info ? kind_info->name : "unknown",
>
> Ugh. Is that something worth a backpatch? Why does this new failure
> mode matter?

Removed in v4. This is not a failure mode we should expect on this
path, so the fallback was just defensive noise.

> + if (kind < PGSTAT_KIND_MIN || kind > PGSTAT_KIND_MAX ||
> + pgStatLocal.kind_hash[kind] == NULL)
>
> In pgstat_drop_entry(), it seems to me that this is a no-op, as in a
> should-never-happen scenario.

Removed in v4.

> Rather than a boolean given to the input function and a GUC, just
> specify a string with a name. I'd imagine a point where a third
> variable-sized custom kind gets introduced. Perhaps also assign a
> different name to the new custom_stats_own_hash? Relying on one kind
> ID for this module is a good idea. That's less bits eaten in the
> available range.

Done in v4. I replaced the boolean GUC with
test_custom_var_stats.kind, so the test selects a custom stats kind by
name. The TAP test now uses a Perl array with the two names and expected
own_hash values. More cases can be added to the array later while still
sharing the same custom kind ID.

> Perhaps use a generate_series() to reduce the number of queries and
> the number of duplicated lines?

Done.

> No need for a catversion bump in patches posted for reviews.

Removed the catversion bump from v4.

> The change for pg_stat_get_kind_info could be split into its own
> patch.

Done. v4 is split into two patches.

--
Sami

Attachment Content-Type Size
v4-0002-pgstat-expose-own_hash-in-pg_stat_kind_info.patch application/octet-stream 5.8 KB
v4-0001-pgstat-allow-a-stats-kind-to-use-its-own-dedicate.patch application/octet-stream 57.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message surya poondla 2026-10-07 22:00:54 Re: Wrong LSN in WAL decoding error messages
Previous Message Zsolt Parragi 2026-10-07 21:44:35 Re: [PROPOSAL] Expand OR clauses in joins to UNION ALL paths