Re: SLRU statistics - Mailing list pgsql-hackers
From | Fujii Masao |
---|---|
Subject | Re: SLRU statistics |
Date | |
Msg-id | b8784fe6-1401-ab35-aa14-d57b5bb8e312@oss.nttdata.com Whole thread Raw |
In response to | Re: SLRU statistics (Fujii Masao <masao.fujii@oss.nttdata.com>) |
Responses |
Re: SLRU statistics
|
List | pgsql-hackers |
On 2020/05/07 13:47, Fujii Masao wrote: > > > On 2020/05/03 1:59, Tomas Vondra wrote: >> On Sat, May 02, 2020 at 12:55:00PM +0200, Tomas Vondra wrote: >>> On Sat, May 02, 2020 at 03:56:07PM +0900, Fujii Masao wrote: >>>> >>>> ... >>> >>>>>> Another thing I found is; pgstat_send_slru() should be called also by >>>>>> other processes than backend? For example, since clog data is flushed >>>>>> basically by checkpointer, checkpointer seems to need to send slru stats. >>>>>> Otherwise, pg_stat_slru.flushes would not be updated. >>>>>> >>>>> >>>>> Hmmm, that's a good point. If I understand the issue correctly, the >>>>> checkpointer accumulates the stats but never really sends them because >>>>> it never calls pgstat_report_stat/pgstat_send_slru. That's only called >>>>> from PostgresMain, but not from CheckpointerMain. >>>> >>>> Yes. >>>> >>>>> I think we could simply add pgstat_send_slru() right after the existing >>>>> call in CheckpointerMain, right? >>>> >>>> Checkpointer sends off activity statistics to the stats collector in >>>> two places, by calling pgstat_send_bgwriter(). What about calling >>>> pgstat_send_slru() just after pgstat_send_bgwriter()? >>>> >>> >>> Yep, that's what I proposed. >>> >>>> In previous email, I mentioned checkpointer just as an example. >>>> So probably we need to investigate what process should send slru stats, >>>> other than checkpointer. I guess that at least autovacuum worker, >>>> logical replication walsender and parallel query worker (maybe this has >>>> been already covered by parallel query some mechanisms. Sorry I've >>>> not checked that) would need to send its slru stats. >>>> >>> >>> Probably. Do you have any other process type in mind? > > No. For now what I'm in mind are just checkpointer, autovacuum worker, > logical replication walsender and parallel query worker. Seems logical > replication worker and syncer have sent slru stats via pgstat_report_stat(). > >> I've looked at places calling pgstat_send_* functions, and I found >> thsese places: >> >> src/backend/postmaster/bgwriter.c >> >> - AFAIK it merely writes out dirty shared buffers, so likely irrelevant. >> >> src/backend/postmaster/checkpointer.c >> >> - This is what we're already discussing here. >> >> src/backend/postmaster/pgarch.c >> >> - Seems irrelevant. >> >> >> I'm a bit puzzled why we're not sending any stats from walsender, which >> I suppose could do various stuff during logical decoding. > > Not sure why, but that seems an oversight... > > > Also I found another minor issue; SLRUStats has not been initialized to 0 > and which could update the counters unexpectedly. Attached patch fixes > this issue. This is minor issue, but basically it's better to fix that before v13 beta1 release. So barring any objection, I will commit the patch. + values[8] = Int64GetDatum(stat.stat_reset_timestamp); Also I found another small issue: pg_stat_get_slru() returns the timestamp when pg_stat_slru was reset by using Int64GetDatum(). This works maybe because the timestamp is also int64. But TimestampTzGetDatum() should be used here, instead. Patch attached. Thought? Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Attachment
pgsql-hackers by date: