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