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
