|From:||Andres Freund <andres(at)anarazel(dot)de>|
|To:||Magnus Hagander <magnus(at)hagander(dot)net>|
|Cc:||Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Georgios <gkokolatos(at)protonmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>|
|Subject:||Re: shared-memory based stats collector|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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,
- 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.
|Next Message||Mark Dilger||2021-03-16 02:10:37||Re: pg_amcheck contrib application|
|Previous Message||Thomas Munro||2021-03-16 01:41:14||Re: Consider Parallelism While Planning For REFRESH MATERIALIZED VIEW|