Re: About to add WAL write/fsync statistics to pg_stat_wal view

On 2021-02-08 13:01, Fujii Masao wrote:
> On 2021/02/05 8:45, Masahiro Ikeda wrote:
>> I pgindented the patches.
> 
> Thanks for updating the patches!

Thanks for checking the patches.

> +       <function>XLogWrite</function>, which nomally called by an
> +       <function>issue_xlog_fsync</function>, which nomally called by 
> an
> 
> Typo: "nomally" should be "normally"?

Yes, I'll fix it.

> +       <function>XLogFlush</function> request(see <xref
> linkend="wal-configuration"/>)
> +       <function>XLogFlush</function> request(see <xref
> linkend="wal-configuration"/>),
> 
> Isn't it better to add a space character just after "request"?

Thanks, I'll fix it.

> +					INSTR_TIME_SET_CURRENT(duration);
> +					INSTR_TIME_SUBTRACT(duration, start);
> +					WalStats.m_wal_write_time = INSTR_TIME_GET_MICROSEC(duration);
> 
> If several cycles happen in the do-while loop, m_wal_write_time should 
> be
> updated with the sum of "duration" in those cycles instead of 
> "duration"
> in the last cycle? If yes, "+=" should be used instead of "=" when 
> updating
> m_wal_write_time?
> +			INSTR_TIME_SET_CURRENT(duration);
> +			INSTR_TIME_SUBTRACT(duration, start);
> +			WalStats.m_wal_sync_time = INSTR_TIME_GET_MICROSEC(duration);
> 
> Also "=" should be "+=" in the above?

Yes, they are my mistake when changing the unit from milliseconds to 
microseconds.

Regards,
-- 
Masahiro Ikeda
NTT DATA CORPORATION