Re: WAL usage calculation patch

On Tue, Apr 7, 2020 at 2:48 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Tue, Apr 7, 2020 at 4:36 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Apr 6, 2020 at 7:58 PM Euler Taveira
> > <euler.taveira@2ndquadrant.com> wrote:
> > >
> > > On Mon, 6 Apr 2020 at 10:37, Julien Rouhaud <rjuju123@gmail.com> wrote:
> > >>
> > >> On Mon, Apr 06, 2020 at 10:12:55AM -0300, Euler Taveira wrote:
> > >> > On Mon, 6 Apr 2020 at 00:25, Amit Kapila <amit.kapila16@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.
> > >> > >
> > >> > > I wasn't paying much attention to this thread. May I suggest changing
> > >> > wal_num_fpw to wal_fpw? wal_records and wal_bytes does not have a prefix
> > >> > 'num'. It seems inconsistent to me.
> > >> >
> > >>
> > >> If we want to be consistent shouldn't we rename it to wal_fpws?  FTR I don't
> > >> like much either version.
> > >
> > >
> > > Since FPW is an acronym, plural form reads better when you are using uppercase (such as FPWs or FPW's); thus, I
prefersingular form because parameter names are lowercase. Function description will clarify that this is "number of
WALfull page writes". 
> > >
> >
> > I like Euler's suggestion to change wal_num_fpw to wal_fpw.  It is
> > better if others who didn't like this name can also share their
> > opinion now because changing multiple times the same thing is not a
> > good idea.
>
> +1
>
> About Justin and your comments on the other thread:
>
> On Tue, Apr 7, 2020 at 4:31 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Apr 6, 2020 at 10:04 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > >
> > > On Thu, Apr 02, 2020 at 08:29:31AM +0200, Julien Rouhaud wrote:
> > > > > > "full page records" seems to be showing the number of full page
> > > > > > images, not the record having full page images.
> > > > >
> > > > > I am not sure what exactly is a difference but it is the records
> > > > > having full page images.  Julien correct me if I am wrong.
> > >
> > > > Obviously previous complaints about the meaning and parsability of
> > > > "full page writes" should be addressed here for consistency.
> > >
> > > There's a couple places that say "full page image records" which I think is
> > > language you were trying to avoid.  It's the number of pages, not the number of
> > > records, no ?  I see explain and autovacuum say what I think is wanted, but
> > > these say the wrong thing?  Find attached slightly larger patch.
> > >
> > > $ git grep 'image record'
> > > contrib/pg_stat_statements/pg_stat_statements.c:        int64           wal_num_fpw;    /* # of WAL full page
imagerecords generated */ 
> > > doc/src/sgml/ref/explain.sgml:      number of records, number of full page image records and amount of WAL
> > >
> >
> > Few comments:
> > 1.
> > - int64 wal_num_fpw; /* # of WAL full page image records generated */
> > + int64 wal_num_fpw; /* # of WAL full page images generated */
> >
> > Let's change comment as " /* # of WAL full page writes generated */"
> > to be consistent with other places like instrument.h.  Also, make a
> > similar change at other places if required.
>
> Agreed.  That's pg_stat_statements.c and instrument.h.  I'll send a
> patch once we reach consensus with the rest of the comments.
>

Would you like to send a consolidated patch that includes Euler's
suggestion and Justin's patch (by making changes for points we
discussed.)?  I think we can keep the point related to number of
spaces before each field open?

> > 2.
> >        <entry>
> > -        Total amount of WAL bytes generated by the statement
> > +        Total number of WAL bytes generated by the statement
> >        </entry>
> >
> > I feel the previous text was better as this field can give us the size
> > of WAL with which we can answer "how much WAL data is generated by a
> > particular statement?".  Julien, do you have any thoughts on this?
>
> I also prefer "amount" as it feels more natural.
>

As we see no other opinion on this matter, we can use "amount" here.

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