> 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)
v3-0001-pgstat-allow-a-stats-kind-to-use-its-own-dedicate.patch
Description: Binary data
