Horton

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

David G. Johnston <david.g.johnston@gmail.com>

View thread View on postgr.es
From: "David G. Johnston" <david.g.johnston@gmail.com>
To: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Li Japin <japinli@hotmail.com>, kuroda.hayato@fujitsu.com
Date: 2021-01-25T23:52:44Z
Lists: pgsql-hackers 
On Mon, Jan 25, 2021 at 4:37 PM Masahiro Ikeda <ikedamsh@oss.nttdata.com>
wrote:

>
> I agree with your comments. I think it should report when
> reaching the end of WAL too. I add the code to report the stats
> when finishing the current WAL segment file when timeout in the
> main loop and when reaching the end of WAL.
>
>
The following is not an improvement:

- /* Send WAL statistics to the stats collector. */
+ /* Send WAL statistics to stats collector */

The word "the" there makes it proper English.  Your copy-pasting should
have kept the existing good wording in the other locations rather than
replace the existing location with the newly added incorrect wording.

This doesn't make sense:

* current WAL segment file to avoid loading stats collector.

Maybe "overloading" or "overwhelming"?

I see you removed the pgstat_send_wal(force) change.  The rest of my
comments on the v6 patch still stand I believe.

David J.

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.