From: | Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | Julien Rouhaud <rjuju123(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pg_stat_statements issue with parallel maintenance (Was Re: WAL usage calculation patch) |
Date: | 2020-03-31 05:13:34 |
Message-ID: | CA+fd4k5L4yVoWz0smymmqB4_SMHd2tyJExUgA_ACsL7k00B5XQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, 31 Mar 2020 at 12:58, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Mar 30, 2020 at 12:31 PM Masahiko Sawada
> <masahiko(dot)sawada(at)2ndquadrant(dot)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(at)gmail(dot)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
Attachment | Content-Type | Size |
---|---|---|
bufferusage_vacuum_v3.patch | application/octet-stream | 4.5 KB |
bufferusage_create_index_v2.patch | application/octet-stream | 4.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro Horiguchi | 2020-03-31 05:20:16 | Re: [HACKERS] Restricting maximum keep segments by repslots |
Previous Message | Kyotaro Horiguchi | 2020-03-31 05:10:14 | Re: [HACKERS] Restricting maximum keep segments by repslots |