Re: Refactor calculations to use instr_time

From: Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: andres(at)anarazel(dot)de, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Refactor calculations to use instr_time
Date: 2023-02-21 13:11:19
Message-ID: CAN55FZ2p=bW5hTPT_8LYcrKNvtcdhYNxrDgTjqJF7YNX9JSaEw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Thanks for the review.

On Mon, 20 Feb 2023 at 06:01, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Fri, 17 Feb 2023 13:53:36 +0300, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> wrote in
> > Thanks for the review. I updated the patch.
>
>
> WalUsageAccumDiff(&diff, &pgWalUsage, &prevWalUsage);
> - PendingWalStats.wal_records = diff.wal_records;
> - PendingWalStats.wal_fpi = diff.wal_fpi;
> - PendingWalStats.wal_bytes = diff.wal_bytes;
> ...
> + WALSTAT_ACC(wal_records, diff);
> + WALSTAT_ACC(wal_fpi, diff);
> + WALSTAT_ACC(wal_bytes, diff);
> + WALSTAT_ACC(wal_buffers_full, PendingWalStats);
>
>
> The lifetime of the variable "diff" seems to be longer now. Wouldn't
> it be clearer if we renamed it to something more meaningful, like
> wal_usage_diff, WalUsageDiff or PendingWalUsage? Along those same
> lines, it occurs to me that the new struct should be named
> PgStat_PendingWalStats, instead of ..Usage. That change makes the name
> of the type and the variable consistent.

I agree. The patch is updated.

Regards,
Nazir Bilal Yavuz
Microsoft

Attachment Content-Type Size
v3-0001-Refactor-instr_time-calculations.patch application/octet-stream 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2023-02-21 13:48:00 Re: Allow logical replication to copy tables in binary format
Previous Message Andrew Dunstan 2023-02-21 12:47:13 Re: meson logs environment