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
