At Tue, 21 Feb 2023 16:11:19 +0300, Nazir Bilal Yavuz <byavu...@gmail.com> wrote in > I agree. The patch is updated.
Thanks, that part looks good to me. I'd like to provide some additional comments. PgStat_PendingStats should be included in typedefs.list. + * Created for accumulating wal_write_time and wal_sync_time as a instr_time + * but instr_time can't be used as a type where it ends up on-disk + * because its units may change. PgStat_WalStats type is used for + * in-memory/on-disk data. So, PgStat_PendingWalStats is created for + * accumulating intervals as a instr_time. + */ +typedef struct PgStat_PendingWalStats IMHO, this comment looks somewhat off. Maybe we could try something like the following instead? > This struct stores wal-related durations as instr_time, which makes it > easier to accumulate them without requiring type conversions. Then, > during stats flush, they will be moved into shared stats with type > conversions. The aim of this patch is to keep using instr_time for accumulation. So it seems like we could do the same refactoring for pgStatBlockReadTime, pgStatBlockWriteTime, pgStatActiveTime and pgStatTransactionIdleTime. What do you think - should we include this additional refactoring in the same patch or make a separate one for it? regards. -- Kyotaro Horiguchi NTT Open Source Software Center
