Horton

Re: Show WAL write and fsync stats in pg_stat_io

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Nazir Bilal Yavuz <byavuz81@gmail.com>
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-30T07:37:46Z
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

On Wed, Jan 29, 2025 at 02:57:21PM +0300, Nazir Bilal Yavuz wrote:
> On Tue, 28 Jan 2025 at 07:23, Michael Paquier <michael@paquier.xyz> wrote:
>> What you doing in 0001 is a first good step towards this goal, as this
>> also plugs in a few things for backend statistics with the calls to
>> pgstat_count_io_op[_time]().
> 
> I agree. Do you think that we need to do this simplification in this
> thread or does it need its own thread?

As far as I understand, the simplifications in PgStat_PendingWalStats
require the changes of this thread first, so keeping them around for
now sounds OK to me.

> I agree with you but it was discussed before in this thread [2]. It
> was decided to use both track_wal_io_timing and track_io_timing
> because of the overhead that track_wal_io_timing creates but we can
> still re-discuss it. Do you think that this discussion needs its own
> thread?

Let's decide it on this thread.  You have done a benchmark with fsync
disabled for something that only stresses WAL.  And it is very
dependent on the clock source.  Would you really see a difference
under a normal pgbench workload?  For example, should we compare HEAD 
and the patch with track_io_timing=on but track_wal_io_timing=off
with a modified version of the patch so as IOOBJECT_WAL timing data is 
controlled by track_io_timing=on?  The previous results could have
been also influenced by the timings of pg_stat_wal because
track_wal_io_timing was on.

> If we continue to discuss it in this thread, I am in favor of removing
> track_wal_io_timing and using track_io_timing for all types of I/Os.
> Like you said, this cross-dependency makes things more complex than
> they used to be. Downside of removing track_wal_io_timing is affecting
> people who:
> 
> 1- Want to track timings of only WAL I/Os.
> 2- Want to track timings of all IOs except WAL I/Os.
> 
> I think the first group is more important than the second because
> track_io_timing already creates overhead.
> 
> One additional thing is that I think track_io_timing is a general
> word. When it exists, I do not expect there to be another GUC like
> track_wal_io_timing to track WAL I/Os' timings.

Just to be clear here, I'd be okay to remove entirely the GUC
track_wal_io_timing iff pg_stat_wal has no more need for it if we feed
the data of pg_stat_io to pg_stat_wal.  Having track_io_timing be
used for all the timing information in pg_stat_io makes the whole
design leaner, IMO, removing it from the patch and pgstat_io.c
simplifies a lot the user history.
--
Michael