Horton

Re: WAL usage calculation patch

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Euler Taveira <euler.taveira@2ndquadrant.com>, Dilip Kumar <dilipbalaut@gmail.com>, Kuntal Ghosh <kuntalghosh.2007@gmail.com>, Fujii Masao <masao.fujii@oss.nttdata.com>, Kirill Bychik <kirill.bychik@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@gmail.com>
Date: 2020-04-10T06:16:46Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Change the display of WAL usage statistics in Explain.

  2. Cosmetic fixups for WAL usage work.

  3. Allow parallel create index to accumulate buffer usage stats.

  4. Allow autovacuum to log WAL usage statistics.

  5. Add the option to report WAL usage in EXPLAIN and auto_explain.

  6. Allow pg_stat_statements to track WAL usage statistics.

  7. Add infrastructure to track WAL usage.

  8. Include information on buffer usage during planning phase, in EXPLAIN output, take two.

  9. Include information on buffer usage during planning phase, in EXPLAIN output.

  10. Allow parallel vacuum to accumulate buffer usage.

  11. Allow pg_stat_statements to track planning statistics.

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 prefer singular form because parameter names are lowercase. Function description will clarify that this is "number of WAL full 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 image records 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