]> granicus.if.org Git - postgresql/commitdiff
Fix buffer usage stats for parallel nodes.
authorAmit Kapila <akapila@postgresql.org>
Fri, 3 Aug 2018 05:57:11 +0000 (11:27 +0530)
committerAmit Kapila <akapila@postgresql.org>
Fri, 3 Aug 2018 05:57:11 +0000 (11:27 +0530)
The buffer usage stats is accounted only for the execution phase of the
node.  For Gather and Gather Merge nodes, such stats are accumulated at
the time of shutdown of workers which is done after execution of node due
to which we missed to account them for such nodes.  Fix it by treating
nodes as running while we shut down them.

We can also miss accounting for a Limit node when Gather or Gather Merge
is beneath it, because it can finish the execution before shutting down
such nodes.  So we allow a Limit node to shut down the resources before it
completes the execution.

In the passing fix the gather node code to allow workers to shut down as
soon as we find that all the tuples from the workers have been retrieved.
The original code use to do that, but is accidently removed by commit
01edb5c7fc.

Reported-by: Adrien Nayrat
Author: Amit Kapila and Robert Haas
Reviewed-by: Robert Haas and Andres Freund
Backpatch-through: 9.6 where this code was introduced
Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info

src/backend/executor/execProcnode.c
src/backend/executor/nodeGather.c
src/backend/executor/nodeLimit.c

index 36d2914249c42f9905a4aad3caa70e00cce63988..06c4829b5a257d23ba89be2ca296c9d3964e9174 100644 (file)
@@ -737,6 +737,19 @@ ExecShutdownNode(PlanState *node)
 
        planstate_tree_walker(node, ExecShutdownNode, NULL);
 
+       /*
+        * Treat the node as running while we shut it down, but only if it's run
+        * at least once already.  We don't expect much CPU consumption during
+        * node shutdown, but in the case of Gather or Gather Merge, we may shut
+        * down workers at this stage.  If so, their buffer usage will get
+        * propagated into pgBufferUsage at this point, and we want to make sure
+        * that it gets associated with the Gather node.  We skip this if the node
+        * has never been executed, so as to avoid incorrectly making it appear
+        * that it has.
+        */
+       if (node->instrument && node->instrument->running)
+               InstrStartNode(node->instrument);
+
        switch (nodeTag(node))
        {
                case T_GatherState:
@@ -755,5 +768,9 @@ ExecShutdownNode(PlanState *node)
                        break;
        }
 
+       /* Stop the node if we started it above, reporting 0 tuples. */
+       if (node->instrument && node->instrument->running)
+               InstrStopNode(node->instrument, 0);
+
        return false;
 }
index 597cbfaa16d21dcee506e6d299d45ce5d2a43943..48633b965eca2c5e315816d6418fb7044e4fcb1e 100644 (file)
@@ -327,7 +327,10 @@ gather_readnext(GatherState *gatherstate)
                        Assert(!tup);
                        --gatherstate->nreaders;
                        if (gatherstate->nreaders == 0)
+                       {
+                               ExecShutdownGatherWorkers(gatherstate);
                                return NULL;
+                       }
                        memmove(&gatherstate->reader[gatherstate->nextreader],
                                        &gatherstate->reader[gatherstate->nextreader + 1],
                                        sizeof(TupleQueueReader *)
index ac5a2ff0e601274a1f2868b93527660caef79321..cf1851e235f9f2f636aeecf09713fce4c622594a 100644 (file)
@@ -134,6 +134,8 @@ ExecLimit(PlanState *pstate)
                                        node->position - node->offset >= node->count)
                                {
                                        node->lstate = LIMIT_WINDOWEND;
+                                       /* Allow nodes to release or shut down resources. */
+                                       (void) ExecShutdownNode(outerPlan);
                                        return NULL;
                                }