Re: Refactor calculations to use instr_time

At Tue, 21 Feb 2023 16:11:19 +0300, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)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