Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Julien Rouhaud <rjuju123@gmail.com>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-04-01T02:46:26Z
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, Mar 31, 2020 at 7:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> While testing I have found one issue.  Basically, during a parallel
> vacuum, it was showing more number of
> shared_blk_hits+shared_blks_read.  After, some investigation, I found
> that during the cleanup phase nworkers are -1, and because of this we
> didn't try to launch worker but "lps->pcxt->nworkers_launched" had the
> old launched worker count and shared memory also had old buffer read
> data which was never updated as we did not try to launch the worker.
>
> diff --git a/src/backend/access/heap/vacuumlazy.c
> b/src/backend/access/heap/vacuumlazy.c
> index b97b678..5dfaf4d 100644
> --- a/src/backend/access/heap/vacuumlazy.c
> +++ b/src/backend/access/heap/vacuumlazy.c
> @@ -2150,7 +2150,8 @@ lazy_parallel_vacuum_indexes(Relation *Irel,
> IndexBulkDeleteResult **stats,
>          * Next, accumulate buffer usage.  (This must wait for the workers to
>          * finish, or we might get incomplete data.)
>          */
> -       for (i = 0; i < lps->pcxt->nworkers_launched; i++)
> +       nworkers = Min(nworkers, lps->pcxt->nworkers_launched);
> +       for (i = 0; i < nworkers; i++)
>                 InstrAccumParallelQuery(&lps->buffer_usage[i]);
>
> It worked after the above fix.
>

Good catch.  I think we should not even call
WaitForParallelWorkersToFinish for such a case.  So, I guess the fix
could be,

if (workers > 0)
{
WaitForParallelWorkersToFinish();
for (i = 0; i < lps->pcxt->nworkers_launched; i++)
                 InstrAccumParallelQuery(&lps->buffer_usage[i]);
}

or something along those lines.

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