Horton

Re: Explain buffers wrong counter with parallel plans

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Amit Kapila <amit.kapila16@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-02T12:32:45Z
Lists: pgsql-hackers 
On Sun, Jun 10, 2018 at 1:18 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> Right, I think we have following options:
> (a) Come up with a solution which allows percolating the buffer usage
> and or similar stats to upper nodes in all cases.
> (b) Allow it to work for some of the cases as it was earlier.
>
> I think (b) can cause confusion and could lead to further questions on
> which specific cases will it work and for which it won't work.  If we
> think (a) is a reasonable approach, then we can close this item with a
> conclusion as a work item for future and OTOH if we think option (b)
> is the better way to deal with it, then we can come up with a patch to
> do so.  My inclination is to go with option (a), but I don't mind if
> the decision is to choose option (b).

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.

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.

We can fix other cases as we find them.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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.