public inbox for [email protected]  

help / color / mirror / Atom feed

From: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Refactor calculations to use instr_time
Date: Wed, 22 Feb 2023 11:50:19 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAN55FZ2p=bW5hTPT_8LYcrKNvtcdhYNxrDgTjqJF7YNX9JSaEw@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CAN55FZ2p=bW5hTPT_8LYcrKNvtcdhYNxrDgTjqJF7YNX9JSaEw@mail.gmail.com>

At Tue, 21 Feb 2023 16:11:19 +0300, Nazir Bilal Yavuz <[email protected]> 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

view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Refactor calculations to use instr_time
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox