Re: WAL usage calculation patch

On Sat, Apr 4, 2020 at 2:50 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>
> On Sat, Apr 04, 2020 at 02:39:32PM +0530, Amit Kapila wrote:
> > On Sat, Apr 4, 2020 at 2:24 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> > > >
> > > > We can add if we want but I am not able to convince myself for that.
> > > > Do you have any use case in mind? I think in most of the cases
> > > > (except for hint-bit WAL) it will be zero. If we are not sure of this
> > > > we can also discuss it separately in a new thread once this
> > > > patch-series is committed and see if anybody else sees the value of it
> > > > and if so adding the code should be easy.
> > >
> > >
> > > I'm mostly thinking of people trying to investigate possible slowdowns on a
> > > hot-standby replica with a primary without wal_log_hints. If they explicitly
> > > ask for WAL information, we should provide them, even if it's quite unlikely to
> > > happen.
> > >
> >
> > Yeah, possible but I am not completely sure. I would like to hear the
> > opinion of others if any before adding code for this. How about if we
> > first commit pg_stat_statements and wait for this till Monday and if
> > nobody responds we can commit the current patch but would start a new
> > thread and try to get the opinion of others?
>
>
> I'm fine with it.
>

I have pushed pg_stat_statements and Explain related patches. I am
now looking into (auto)vacuum patch and have few comments.

@@ -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.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com