Re: Show WAL write and fsync stats in pg_stat_io

Nazir Bilal Yavuz <byavuz81@gmail.com>

From: Nazir Bilal Yavuz <byavuz81@gmail.com>

To: Michael Paquier <michael@paquier.xyz>

Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Melanie Plageman <melanieplageman@gmail.com>, Andres Freund <andres@anarazel.de>, pgsql-hackers <pgsql-hackers@postgresql.org>, "bharath.rupireddyforpostgres@gmail.com" <bharath.rupireddyforpostgres@gmail.com>, Thomas Munro <thomas.munro@gmail.com>

Date: 2025-01-27T13:59:34Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Fix copy-paste error related to the autovacuum launcher in pgstat_io.c
- 17d8bba6dad1 18.0 landed
Move SQL tests of pg_stat_io for WAL data to recovery test 029_stats_restart
- 428fadb7e97e 18.0 landed
Add data for WAL in pg_stat_io and backend statistics
- a051e71e28a1 18.0 landed
Improve comment on top of pgstat_count_io_op_time()
- b998fedab74c 18.0 landed
Refactor pgstat_prepare_io_time() with an input argument instead of a GUC
- 3c9d9acae0bc 17.0 landed

Attachments

v11-0001-Add-WAL-I-O-stats-to-both-pg_stat_io-view-and-pe.patch (text/x-patch) patch v11-0001
v11-0002-Fetch-timing-columns-from-pg_stat_io-in-the-pg_s.patch (text/x-patch) patch v11-0002

Hi,

On Mon, 27 Jan 2025 at 03:52, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Fri, Jan 24, 2025 at 06:29:46PM +0300, Nazir Bilal Yavuz wrote:
> > By saying that, this does not solve the problem; there is still a
> > difference although you omit WAL initialization stats from the
> > pg_stat_io.
>
> Reattaching the remaining patches to make the cfbot happy, as 0001 has
> been applied as 87a6690cc695.

Thanks!

>
> -                if (track_wal_io_timing)
> -                {
> -                    instr_time    end;
> -
> -                    INSTR_TIME_SET_CURRENT(end);
> -                    INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_write_time, end, start);
> -                }
>
> In 0002, you are removing PendingWalStats.wal_write_time, which does
> not seem OK to me because we should still aggregate this data for
> track_wal_io_timing, no?

We use PendingWalStats.wal_[ write | sync ]_time only to show timings
on the pg_stat_wal view, right? And now these pg_stat_wal.wal_[ write
| sync ]_time datas are fetched from the pg_stat_io view when the
track_wal_io_timing is on. So, I think it is correct to remove these.

I made a mistake while splitting the patches. The places where
'PendingWalStats.wal_[ write | sync ]_time are incremented (the code
piece you shared)' are removed in 0002 (0001 now), but they should be
removed in 0003 (0002 now) instead. This is corrected in v11.

--
Regards,
Nazir Bilal Yavuz
Microsoft