Re: shared-memory based stats collector

Hi,

On 2021-03-13 10:05:21 -0800, Andres Freund wrote:
> Cool. I'll give it a try.

I have a few questions about the patch:

- Why was collect_oids() changed to a different hashtable as part of
this change? Seems fairly independent?

- What's the point of all those cached_* stuff? There's not a single
comment explaining it as far as I can tell...

Several of them are never used as a cache! E.g. cached_archiverstats,
cached_bgwriterstats, ...

- What is the idea behind pgstat_reset_shared_counters() using
pgstat_copy_global_stats() to reset, using StatsShmem->*_reset_offset?
But then still taking a lock in pgstat_fetch_stat_*? Again, no
comments explaining what the goal is.

It kinda looks like you tried to make both read and write paths not
use the lock, but then ended up using a lock?

Do you have some benchmarks that you used to verify performance?

I think I'm going to try to split the storage of fixed-size stats in
StatsShmemStruct into a separate patch. That's already a pretty large
change, and it's pretty much unrelated to the rest.

Greetings,

Andres Freund