Horton

Re: pg_stat_statements issue with parallel maintenance (Was Re: WAL usage calculation patch)

Masahiko Sawada <masahiko.sawada@2ndquadrant.com>

View thread
From: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-04-06T05:48:39Z
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.

Attachments

On Tue, 31 Mar 2020 at 14:13, Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
>
> On Tue, 31 Mar 2020 at 12:58, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Mar 30, 2020 at 12:31 PM Masahiko Sawada
> > <masahiko.sawada@2ndquadrant.com> wrote:
> > >
> > > The patch for vacuum conflicts with recent changes in vacuum. So I've
> > > attached rebased one.
> > >
> >
> > + /*
> > + * Next, accumulate buffer usage.  (This must wait for the workers to
> > + * finish, or we might get incomplete data.)
> > + */
> > + for (i = 0; i < nworkers; i++)
> > + InstrAccumParallelQuery(&lps->buffer_usage[i]);
> > +
> >
> > This should be done for launched workers aka
> > lps->pcxt->nworkers_launched.  I think a similar problem exists in
> > create index related patch.
>
> You're right. Fixed in the new patches.
>
> On Mon, 30 Mar 2020 at 17:00, Julien Rouhaud <rjuju123@gmail.com> wrote:
> >
> > Just minor nitpicking:
> >
> > +   int         i;
> >
> >     Assert(!IsParallelWorker());
> >     Assert(ParallelVacuumIsActive(lps));
> > @@ -2166,6 +2172,13 @@ lazy_parallel_vacuum_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
> >     /* Wait for all vacuum workers to finish */
> >     WaitForParallelWorkersToFinish(lps->pcxt);
> >
> > +   /*
> > +    * Next, accumulate buffer usage.  (This must wait for the workers to
> > +    * finish, or we might get incomplete data.)
> > +    */
> > +   for (i = 0; i < nworkers; i++)
> > +       InstrAccumParallelQuery(&lps->buffer_usage[i]);
> >
> > We now allow declaring a variable in those loops, so it may be better to avoid
> > declaring i outside the for scope?
>
> We can do that but I was not sure if it's good since other codes
> around there don't use that. So I'd like to leave it for committers.
> It's a trivial change.
>

I've updated the buffer usage patch for parallel index creation as the
previous patch conflicts with commit df3b181499b40.

This comment in commit df3b181499b40 seems the comment which had been
replaced by Amit with a better sentence when introducing buffer usage
to parallel vacuum.

+   /*
+    * Estimate space for WalUsage -- PARALLEL_KEY_WAL_USAGE
+    *
+    * WalUsage during execution of maintenance command can be used by an
+    * extension that reports the WAL usage, such as pg_stat_statements. We
+    * have no way of knowing whether anyone's looking at pgWalUsage, so do it
+    * unconditionally.
+    */

Would the following sentence in lazyvacuum.c be also better for
parallel create index?

    * If there are no extensions loaded that care, we could skip this.  We
    * have no way of knowing whether anyone's looking at pgBufferUsage or
    * pgWalUsage, so do it unconditionally.

The attached patch changes to the above comment and removed the code
that is used to un-support only buffer usage accumulation.

Regards,

-- 
Masahiko Sawada            http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services