Hi, Thanks for the review.
On Mon, 20 Feb 2023 at 06:01, Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > At Fri, 17 Feb 2023 13:53:36 +0300, Nazir Bilal Yavuz <byavu...@gmail.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
v3-0001-Refactor-instr_time-calculations.patch
Description: Binary data
