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

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

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-03-31T05:13:34Z

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 →

Change the display of WAL usage statistics in Explain.
- 69bfaf2e1de4 13.0 landed
Cosmetic fixups for WAL usage work.
- ef08ca113fe3 13.0 landed
Allow parallel create index to accumulate buffer usage stats.
- 5c71362174eb 13.0 landed
- f179e9f01b65 11.8 landed
- 13823ad3b012 12.3 landed
Allow autovacuum to log WAL usage statistics.
- b7ce6de93b59 13.0 landed
Add the option to report WAL usage in EXPLAIN and auto_explain.
- 33e05f89c53e 13.0 landed
Allow pg_stat_statements to track WAL usage statistics.
- 6b466bf5f2be 13.0 landed
Add infrastructure to track WAL usage.
- df3b181499b4 13.0 landed
Include information on buffer usage during planning phase, in EXPLAIN output, take two.
- ce77abe63cfc 13.0 cited
Include information on buffer usage during planning phase, in EXPLAIN output.
- ed7a5095716e 13.0 cited
Allow parallel vacuum to accumulate buffer usage.
- 3a5e22138a8d 13.0 landed
Allow pg_stat_statements to track planning statistics.
- 17e03282241c 13.0 cited

Attachments

bufferusage_vacuum_v3.patch (application/octet-stream) patch v3
bufferusage_create_index_v2.patch (application/octet-stream) patch v2

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.

Regards,

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