> 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
v4-0002-pgstat-expose-own_hash-in-pg_stat_kind_info.patch
Description: Binary data
v4-0001-pgstat-allow-a-stats-kind-to-use-its-own-dedicate.patch
Description: Binary data
