Horton

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

Julien Rouhaud <rjuju123@gmail.com>

View thread
From: Julien Rouhaud <rjuju123@gmail.com>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-03-30T08:00:05Z
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 Mon, Mar 30, 2020 at 04:01:18PM +0900, Masahiko Sawada wrote:
> On Mon, 30 Mar 2020 at 15:46, Masahiko Sawada
> <masahiko.sawada@2ndquadrant.com> wrote:
> >
> > On Sun, 29 Mar 2020 at 20:44, Masahiko Sawada
> > <masahiko.sawada@2ndquadrant.com> wrote:
> > >
> > > > > > I think we need to change parallel maintenance commands so that they
> > > > > > report buffer usage like what ParallelQueryMain() does; prepare to
> > > > > > track buffer usage during query execution by
> > > > > > InstrStartParallelQuery(), and report it by InstrEndParallelQuery()
> > > > > > after parallel maintenance command. To report buffer usage of parallel
> > > > > > maintenance command correctly, I'm thinking that we can (1) change
> > > > > > parallel create index and parallel vacuum so that they prepare
> > > > > > gathering buffer usage, or (2) have a common entry point for parallel
> > > > > > maintenance commands that is responsible for gathering buffer usage
> > > > > > and calling the entry functions for individual maintenance command.
> > > > > > I'll investigate it more in depth.
> > > > >
> > > [...]
> >
> > I've attached two patches fixing this issue for parallel index
> > creation and parallel vacuum. These approaches take the same approach;
> > we allocate DSM to share buffer usage and the leader gathers them,
> > described as approach (1) above. I think this is a straightforward
> > approach for this issue. We can create a common entry point for
> > parallel maintenance command that is responsible for gathering buffer
> > usage as well as sharing query text etc. But it will accompany
> > relatively big change and it might be overkill at this stage. We can
> > discuss that and it will become an item for PG14.
> >
> 
> The patch for vacuum conflicts with recent changes in vacuum. So I've
> attached rebased one.

Thanks Sawada-san!

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?

Other than that both patch looks good to me and a good fit for packpatching.  I
also did some testing on VACUUM and CREATE INDEX and it works as expected.