Re: wal stats questions

On 2021/04/21 15:08, torikoshia wrote:
> On 2021-04-16 10:27, Masahiro Ikeda wrote:
>> On 2021/04/13 9:33, Fujii Masao wrote:
>>> 
>>> 
>>> On 2021/03/30 20:37, Masahiro Ikeda wrote:
>>>> OK, I added the condition to the fast-return check. I noticed that I 
>>>> misunderstood that the purpose is to avoid expanding a clock check
>>>> using WAL stats counters. But, the purpose is to make the conditions
>>>> stricter, right?
>>> 
>>> Yes. Currently if the following condition is false even when the WAL
>>> counters are updated, nothing is sent to the stats collector. But with
>>> your patch, in this case the WAL stats are sent.
>>> 
>>> if ((pgStatTabList == NULL || pgStatTabList->tsa_used == 0) && 
>>> pgStatXactCommit == 0 && pgStatXactRollback == 0 && 
>>> !have_function_stats && !disconnect)
>>> 
>>> Thanks for the patch! It now fails to be applied to the master
>>> cleanly. So could you rebase the patch?
>> 
>> Thanks for your comments! I rebased it.
> 
> Thanks for working on this!

Hi, thanks for your comments!


> I have some minor comments on 
> performance-improvements-of-reporting-wal-stats-without-introducing-a-new-variable.patch.
>
> 
> 
> 
> 177 @@ -3094,20 +3066,33 @@ pgstat_report_wal(void) 178   * Return true if
> the message is sent, and false otherwise.
> 
> Since you changed the return value to void, it seems the description is not
> necessary anymore.

Right, I fixed it.


> 208 +        * generate wal records. 'wal_writes' and 'wal_sync' are zero 
> means the
> 
> It may be better to change 'wal_writes' to 'wal_write' since single 
> quotation seems to mean variable name.

Agreed.


> 234 +        * set the counters related to generated WAL data if the
> counters are
> 
> 
> set -> Set?

Yes, I fixed.

> BTW, is it better to change PgStat_Counter from int64 to uint64 because> there aren't any counters with negative number?

Although this is not related to torikoshi-san's comment, the above my
understanding is not right. Some counters like delta_live_tuples,
delta_dead_tuples and changed_tuples can be negative.


Regards,
-- 
Masahiro Ikeda
NTT DATA CORPORATION