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.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


Reply via email to