Subject: Re: SLRU statistics
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
 Masahiko Sawada <masahiko.sawada@2ndquadrant.com>,
 "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
References: <20200229205518.xrxo535zgd7cl6no@development>
 <20200327004111.md4suoi6u5suv2qe@development>
 <20200328002630.2nz5y3dvymqnvxtv@development>
 <20200402004136.wodwftan45mcwggm@development>
 <f02fd0ec-a240-b5bf-9992-5faf582738f1@oss.nttdata.com>
 <20200430181901.24tlriahmafi4sfn@development>
 <0e1c8904-13be-ab70-0096-54f0b30e43ba@oss.nttdata.com>
 <20200502000818.nn4kpewiput42bnb@development>
 <49b8ad17-f6d9-a086-a952-ff1fec3ac1b3@oss.nttdata.com>
 <20200502105500.hwhba3d6s3hlrqf6@development>
 <20200502165905.brfxv6dk5vjdbgti@development>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Message-ID: <976bbb73-a112-de3c-c488-b34b64609793@oss.nttdata.com>
Date: Thu, 7 May 2020 13:47:40 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0)
 Gecko/20100101 Thunderbird/68.8.0
MIME-Version: 1.0
In-Reply-To: <20200502165905.brfxv6dk5vjdbgti@development>
Content-Type: multipart/mixed;
 boundary="------------95AF2B8849AE9CCFFBDAF48D"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------95AF2B8849AE9CCFFBDAF48D
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit


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.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

--------------95AF2B8849AE9CCFFBDAF48D
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
 name="init_slru_stats.patch"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="init_slru_stats.patch"

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3Bvc3RtYXN0ZXIvcGdzdGF0LmMgYi9zcmMvYmFj
a2VuZC9wb3N0bWFzdGVyL3Bnc3RhdC5jCmluZGV4IDNmODEwNWM2ZWIuLjQxNmY4NmZiZDYg
MTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3Bvc3RtYXN0ZXIvcGdzdGF0LmMKKysrIGIvc3Jj
L2JhY2tlbmQvcG9zdG1hc3Rlci9wZ3N0YXQuYwpAQCAtMjkwMCw2ICsyOTAwLDkgQEAgcGdz
dGF0X2luaXRpYWxpemUodm9pZCkKIAkJTXlCRUVudHJ5ID0gJkJhY2tlbmRTdGF0dXNBcnJh
eVtNYXhCYWNrZW5kcyArIE15QXV4UHJvY1R5cGVdOwogCX0KIAorCS8qIEluaXRpYWxpemUg
U0xSVSBzdGF0aXN0aWNzIHRvIHplcm8gKi8KKwltZW1zZXQoJlNMUlVTdGF0cywgMCwgc2l6
ZW9mKFNMUlVTdGF0cykpOworCiAJLyogU2V0IHVwIGEgcHJvY2Vzcy1leGl0IGhvb2sgdG8g
Y2xlYW4gdXAgKi8KIAlvbl9zaG1lbV9leGl0KHBnc3RhdF9iZXNodXRkb3duX2hvb2ssIDAp
OwogfQo=
--------------95AF2B8849AE9CCFFBDAF48D--