Re: SLRU statistics

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: SLRU statistics
Date: 2020-05-13 08:21:59
Message-ID: 20200513082159.myflgcs5fnhnujbk@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, May 13, 2020 at 04:10:30PM +0900, Fujii Masao wrote:
>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.
>>>>>>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:
>>>- AFAIK it merely writes out dirty shared buffers, so likely irrelevant.
>>>- This is what we're already discussing here.
>>>- 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?

I agree with both fixes.


Tomas Vondra
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message 2020-05-13 08:29:19 Re: A patch for get origin from commit_ts.
Previous Message Andy Fan 2020-05-13 08:17:50 Re: Add "-Wimplicit-fallthrough" to default flags