Re: Explain buffers wrong counter with parallel plans

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Adrien Nayrat <adrien(dot)nayrat(at)anayrat(dot)info>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Explain buffers wrong counter with parallel plans
Date: 2018-07-03 10:48:13
Message-ID: CAA4eK1Lumkbhy0W1Ky5uNof2zoCgSBE9WRee+OknuuepgP6sAg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 2, 2018 at 6:02 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> I think the core problem here is this hunk from gather_readnext:
>
> {
> Assert(!tup);
> - DestroyTupleQueueReader(reader);
> --gatherstate->nreaders;
> if (gatherstate->nreaders == 0)
> - {
> - ExecShutdownGatherWorkers(gatherstate);
> return NULL;
> - }
> memmove(&gatherstate->reader[gatherstate->nextreader],
> &gatherstate->reader[gatherstate->nextreader + 1],
> sizeof(TupleQueueReader *)
>
> Since ExecShutdownGatherWorkers() is no longer called there, the
> instrumentation data isn't accumulated into the Gather node when the
> workers are shut down. I think that's a bug and we should fix it.
>

Yeah, previously, I have also pointed out the same code [1]. However,
I have not done any testing to prove it.

> To fix the problem with Limit that you mention, we could just modify
> nodeLimit.c so that when the state is changed from LIMIT_INWINDOW to
> LIMIT_WINDOWEND, we also call ExecShutdownNode on the child plan.
>

It should work.

> We can fix other cases as we find them.
>

I think we have a similar problem with GatherMerge, but that also
appears to be fixable.

Are you planning to work on it? If not, then I can look into it.

[1] - https://www.postgresql.org/message-id/CAA4eK1KZEbYKj9HHP-6WqqjAXuoB%2BWJu-w1s9uovj%3DeeBxC48Q%40mail.gmail.com

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message amul sul 2018-07-03 10:58:30 Re: Failed assertion due to procedure created with SECURITY DEFINER option
Previous Message Surafel Temesgen 2018-07-03 10:31:01 Re: Add --include-table-data-where option to pg_dump, to export only a subset of table data