On Mon, Apr 06, 2020 at 08:55:01AM +0530, Amit Kapila wrote: > On Sat, Apr 4, 2020 at 2:50 PM Julien Rouhaud <rjuju...@gmail.com> wrote: > > > I have pushed pg_stat_statements and Explain related patches. I am > now looking into (auto)vacuum patch and have few comments. >
Thanks! > @@ -614,6 +616,9 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, > > TimestampDifference(starttime, endtime, &secs, &usecs); > > + memset(&walusage, 0, sizeof(WalUsage)); > + WalUsageAccumDiff(&walusage, &pgWalUsage, &walusage_start); > + > read_rate = 0; > write_rate = 0; > if ((secs > 0) || (usecs > 0)) > @@ -666,7 +671,13 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, > (long long) VacuumPageDirty); > appendStringInfo(&buf, _("avg read rate: %.3f MB/s, avg write rate: > %.3f MB/s\n"), > read_rate, write_rate); > - appendStringInfo(&buf, _("system usage: %s"), pg_rusage_show(&ru0)); > + appendStringInfo(&buf, _("system usage: %s\n"), pg_rusage_show(&ru0)); > + appendStringInfo(&buf, > + _("WAL usage: %ld records, %ld full page writes, " > + UINT64_FORMAT " bytes"), > + walusage.wal_records, > + walusage.wal_num_fpw, > + walusage.wal_bytes); > > Here, we are not displaying Buffers related data, so why do we think > it is important to display WAL data? I see some point in displaying > Buffers and WAL data in a vacuum (verbose), but I feel it is better to > make a case for both the statistics together rather than just > displaying one and leaving other. I think the other change related to > autovacuum stats seems okay to me. One thing is that the amount of WAL, and more precisely FPW, is quite unpredictable wrt. vacuum and even more anti-wraparound vacuum, so this is IMHO a very useful metric. That being said I totally agree with you that both should be displayed. Should I send a patch to also expose it?