Re: wal stats questions

On 2021/05/11 18:46, Masahiro Ikeda wrote:
>
>
> On 2021/05/11 16:44, Fujii Masao wrote:
>>
>>
>> On 2021/04/28 9:10, Masahiro Ikeda wrote:
>>>
>>>
>>> On 2021/04/27 21:56, Fujii Masao wrote:
>>>>
>>>>
>>>> On 2021/04/26 10:11, Masahiro Ikeda wrote:
>>>>>
>>>>> First patch has only the changes for pg_stat_wal view.
>>>>> ("v6-0001-performance-improvements-of-reporting-wal-stats-without-introducing-a-new-variable.patch")
>>>>>
>>>>>
>>>>
>>>> +        pgWalUsage.wal_records == prevWalUsage.wal_records &&
>>>> +        walStats.wal_write == 0 && walStats.wal_sync == 0 &&
>>>>> WalStats.m_wal_write should be checked here instead of walStats.wal_write?
>>>
>>> Thanks! Yes, I'll fix it.
>>
>> Thanks!
>
> Thanks for your comments!
> I fixed them.

Thanks for updating the patch!

if ((pgStatTabList == NULL || pgStatTabList->tsa_used == 0) &&
pgStatXactCommit == 0 && pgStatXactRollback == 0 &&
+ pgWalUsage.wal_records == prevWalUsage.wal_records &&
+ WalStats.m_wal_write == 0 && WalStats.m_wal_sync == 0 &&

I'm just wondering if the above WAL activity counters need to be checked.
Maybe it's not necessary because "pgStatXactCommit == 0 && pgStatXactRollback == 0"
is checked? IOW, is there really the case where WAL activity counters are updated
even when both pgStatXactCommit and pgStatXactRollback are zero?

+ if (pgWalUsage.wal_records != prevWalUsage.wal_records)
+ {
+ WalUsage walusage;
+
+ /*
+ * Calculate how much WAL usage counters were increased by substracting
+ * the previous counters from the current ones. Fill the results in
+ * WAL stats message.
+ */
+ MemSet(&walusage, 0, sizeof(WalUsage));
+ WalUsageAccumDiff(&walusage, &pgWalUsage, &prevWalUsage);

Isn't it better to move the code "prevWalUsage = pgWalUsage" into here?
Because it's necessary only when pgWalUsage.wal_records != prevWalUsage.wal_records.

Regards,

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