Skip to content

Commit 19df170

Browse files
author
Amit Kapila
committed
Fix buffer usage stats for parallel nodes.
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 01edb5c. 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/[email protected]
1 parent 7124c93 commit 19df170

File tree

2 files changed

+19
-0
lines changed

2 files changed

+19
-0
lines changed

src/backend/executor/execProcnode.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,19 @@ ExecShutdownNode(PlanState *node)
801801
if (node == NULL)
802802
return false;
803803

804+
/*
805+
* Treat the node as running while we shut it down, but only if it's run
806+
* at least once already. We don't expect much CPU consumption during
807+
* node shutdown, but in the case of Gather or Gather Merge, we may shut
808+
* down workers at this stage. If so, their buffer usage will get
809+
* propagated into pgBufferUsage at this point, and we want to make sure
810+
* that it gets associated with the Gather node. We skip this if the node
811+
* has never been executed, so as to avoid incorrectly making it appear
812+
* that it has.
813+
*/
814+
if (node->instrument && node->instrument->running)
815+
InstrStartNode(node->instrument);
816+
804817
switch (nodeTag(node))
805818
{
806819
case T_GatherState:
@@ -810,5 +823,9 @@ ExecShutdownNode(PlanState *node)
810823
break;
811824
}
812825

826+
/* Stop the node if we started it above, reporting 0 tuples. */
827+
if (node->instrument && node->instrument->running)
828+
InstrStopNode(node->instrument, 0);
829+
813830
return planstate_tree_walker(node, ExecShutdownNode, NULL);
814831
}

src/backend/executor/nodeLimit.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ ExecLimit(LimitState *node)
130130
node->position - node->offset >= node->count)
131131
{
132132
node->lstate = LIMIT_WINDOWEND;
133+
/* Allow nodes to release or shut down resources. */
134+
(void) ExecShutdownNode(outerPlan);
133135
return NULL;
134136
}
135137

0 commit comments

Comments
 (0)