Re: wal stats questions

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!

>
>
>> Is there really the case where the number of sync is larger than zero when
>> the number of writes is zero? If not, it's enough to check only the number
>> of writes?
>
> I thought that there is the case if "wal_sync_method" is fdatasync, fsync or
> fsync_writethrough. The example case is following.
>
> (1) backend-1 writes the wal data because wal buffer has no space. But, it
> doesn't sync the wal data.
> (2) backend-2 reads data pages. In the execution, it need to write and sync
> the wal because dirty pages is selected as victim pages. backend-2 need to
> only sync the wal data because the wal data were already written by backend-1,
> but they weren't synced.

You're right. So let's leave the check of "m_wal_sync == 0".

>
> I'm ok to change it since it's rare case.
>
>
>> +     * wal records weren't generated. So, the counters of 'wal_fpi',
>> +     * 'wal_bytes', 'm_wal_buffers_full' are not updated neither.
>>
>> It's better to add the assertion check that confirms
>> m_wal_buffers_full == 0 whenever wal_records is larger than zero?
>
> Sorry, I couldn't understand yet. I thought that m_wal_buffers_full can be
> larger than 0 if wal_records > 0.
>
> Do you suggest that the following assertion is needed?
>
> - if (memcmp(&WalStats, &all_zeroes, sizeof(PgStat_MsgWal)) == 0)
> - return false;
> + if (pgWalUsage.wal_records == prevWalUsage.wal_records &&
> + WalStats.m_wal_write == 0 && WalStats.m_wal_sync == 0)
> + {
> + Assert(pgWalUsage.wal_fpi == 0 && pgWalUsage.wal_bytes &&
> + WalStats.m_wal_buffers_full == 0 &&
> WalStats.m_wal_write_time == 0 &&
> + WalStats.m_wal_sync_time == 0);
> + return;
> + }

I was thinking to add the "Assert(WalStats.m_wal_buffers_full)" as a safe-guard
because only m_wal_buffers_full is incremented in different places where
wal_records, m_wal_write and m_wal_sync are incremented.

Regards,

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