| 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-06 16:20:16 |
| Message-ID: | CAN12+Y+6eu3=zu7s0evYMW=4Q1j38XffxAf3OFmWmqQ86q20kQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> Having both shared_hash and kind_dsa/kind_hash feels duplicated. Hmm.
> How about enforcing a policy so as kind_dsa/kind_hash are always set
> for variable-sized stats kinds? If kind_hash_valid is true, point
> these to the main shared hashtable instead of using NULL. That would
> make pgstat_get_hash_for_kind() unnecessary. And then perhaps remove
> the shared hashtable pointer all together from the main structure?
Agreed. I have changed this in the attached v3 so that kind_hash[] is
populated for all variable-numbered stats kinds at attach time. Kinds
with own_hash use their dedicated hash, and the others point to the
shared hash. I also removed the shared_hash and dsa fields from
PgStat_LocalState.
I kept pgstat_get_hash_for_kind() as a small inline wrapper around
kind_hash[] for now, mainly to keep the assertion in one place.
> pgstat_get_dsa_for_kind() is a nice wrapper to keep, as that means
> less dshash_get_dsa_area() footprint.
I went back and forth on this one. In the v3 changes, we already have
the selected hash when we need the DSA, so dshash_get_dsa_area(hash)
avoids looking up the hash for the kind again. I can restore the
wrapper if you prefer it for readability, but I prefer this approach
with one less helper.
> It looks like you are right to have this optimization with an
> additional array. That speeds the flush of the stats by not having to
> go through all PGSTAT_KIND_MAX entries, which I suspect could become
> costly with very short queries a-la "SELECT 1".
Thanks, I kept all_hashes[] for that reason. It is built once at attach
time and used by paths that need to scan all variable-numbered stats.
> This code has been moved to pgstat_reset_matching_entries_in_hash(),
> and the corresponding comment is gone. That's important to keep
> documented.
>
> Similar remark here. The exclusive lock part applies to
> dshash_seq_init().
Agreed. I restored both comments in v3.
Other changes in v3:
- Exposed own_hash in pg_stat_kind_info, with the test updated for the
new column.
- The custom stats test now repeats the variable-stats restart/crash
checks for both shared and dedicated hash configurations. It also
adds reset coverage for the variable-stats kind.
--
Sami Imseih
Amazon Web Services (AWS)
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-pgstat-allow-a-stats-kind-to-use-its-own-dedicate.patch | application/octet-stream | 63.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Robert Haas | 2026-10-06 15:53:16 | Re: pgsql: Reduce "Var IS [NOT] NULL" quals during constant folding |