> 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)
