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.
> 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.
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;
+ }
>> Second one has the changes for the type of the BufferUsage's and WalUsage's
>> members. I change the type from long to int64. Is it better to make new
>> thread?
>> ("v6-0002-change-the-data-type-of-XXXUsage-from-long-to-int64.patch")
>
> Will review the patch later. I'm ok to discuss that in this thread.
Thanks!
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
