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

Fujii Masao <masao.fujii@oss.nttdata.com>

From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Masahiro Ikeda <ikedamsh@oss.nttdata.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Li Japin <japinli@hotmail.com>, kuroda.hayato@fujitsu.com, "David G. Johnston" <david.g.johnston@gmail.com>
Date: 2021-02-08T04:01:10Z
Lists: pgsql-hackers

On 2021/02/05 8:45, Masahiro Ikeda wrote:
> I pgindented the patches.

Thanks for updating 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"?

+       <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"?

+					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?

Regards,


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



Commits

  1. Send statistics collected during shutdown checkpoint to the stats collector.

  2. Force to send remaining WAL stats to the stats collector at walwriter exit.

  3. Track total amounts of times spent writing and syncing WAL data to disk.

  4. Retry short writes when flushing WAL.