On Mon, Feb 17, 2025 at 03:14:59PM +0000, Bertrand Drouvot wrote: > PFA the whole picture. 0001 is implementing the fields removal in pg_stat_wal > (and also PendingWalStats). I think that's ok given the backend's type for > which > pgstat_tracks_io_bktype() returns false. But now you make me doubt about 0001.
Double-checking the code now and my doubts are wrong. I think that I would vote for a removal of the fields from pg_stat_wal rather than a replacement in pg_stat_wal, for the following reasons: - pg_stat_stat.wal_write is the same value as "select sum(writes) from pg_stat_io where object = 'wal' and context = 'normal'" as these are incremented in XLogWrite(). - Same argument about pg_stat_wal.wal_write_time with pg_stat_io.write_time. - issue_xlog_fsync() tells that pg_stat_wal.wal_sync_time and sum(pg_stat_io.fsync_time) under object=wal and context=normal are the same values. - Same argument with the fsync counters pg_stat_wal.wal_sync and pg_stat_io.fsyncs. - Encourage monitoring pull to move to pg_stat_io, where there is much more context and granularity of the stats data. Regarding the GUC track_wal_io_timing, my take is that we'll live better if we just let it go. It loses its meaning once pg_stat_wal does not track the write and sync timings. > Anyway, it's probably better to move the 0001 discussion to a dedicated > thread, > thoughts? Yes. And we cannot really move forward with what we have here without deciding about this part. The simplifications I can read from v7-0002~v7-0004 are really nice. These make the implementation of WAL stats at backend-level really simpler to think about. The doc additions of v7-0001 about the description of what the 'wal' object does in pg_stat_io are actually worth a change of their own? We already track them in pg_stat_io. -- Michael
signature.asc
Description: PGP signature
