Horton

Re: Explain buffers wrong counter with parallel plans

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>, Adrien Nayrat <adrien.nayrat@anayrat.info>, Andres Freund <andres@anarazel.de>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Date: 2018-07-03T10:48:13Z
Lists: pgsql-hackers 
On Mon, Jul 2, 2018 at 6:02 PM, Robert Haas <robertmhaas@gmail.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

Commits

  1. Adjust comment atop ExecShutdownNode.

  2. Prohibit shutting down resources if there is a possibility of back up.

  3. Fix buffer usage stats for parallel nodes.

  4. Match the buffer usage tracking for leader and worker backends.

  5. Improve division of labor between execParallel.c and nodeGather[Merge].c.