Re: wal stats questions - Mailing list pgsql-hackers

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