Horton

Re: Show WAL write and fsync stats in pg_stat_io

Nazir Bilal Yavuz <byavuz81@gmail.com>

View thread
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-27T14:13:39Z
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 →

  1. Fix copy-paste error related to the autovacuum launcher in pgstat_io.c

  2. Move SQL tests of pg_stat_io for WAL data to recovery test 029_stats_restart

  3. Add data for WAL in pg_stat_io and backend statistics

  4. Improve comment on top of pgstat_count_io_op_time()

  5. Refactor pgstat_prepare_io_time() with an input argument instead of a GUC

Hi,

On Mon, 27 Jan 2025 at 16:59, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> On Mon, 27 Jan 2025 at 03:52, Michael Paquier <michael@paquier.xyz> wrote:
> >
> > 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.

Oops, I forgot to add one thing to the previous mail.

If we agree with removing PendingWalStats.wal_[ write | sync ]_time
variables, then it would make sense to remove PgStat_PendingWalStats
struct completely. We have that struct because [1] it is cheap to
store PendingWalStats.wal_[ write | sync ]_time as instr_time instead
of PgStat_Counter.

[1] ca7b3c4c00

-- 
Regards,
Nazir Bilal Yavuz
Microsoft