Tim Armstrong has posted comments on this change. Change subject: IMPALA-4862: make resource profile consistent with backend behaviour ......................................................................
Patch Set 5: (5 comments) I found a handful of subtle bugs. Most of them were caught by running tests on my buffer pool development branch and hittings DCHECKs that too many reservations were claimed. The parallel plan bugs were found by inspection: resources required by the parallel plans should always be <= 2x the resources required by the distributed plans. http://gerrit.cloudera.org:8080/#/c/7223/5/be/src/exec/blocking-join-node.cc File be/src/exec/blocking-join-node.cc: Line 156: if (status->ok()) *status = AcquireResourcesForBuild(state); I added AcquireResourcesForBuild() to defer consuming resources until the build side is open. Line 169: if (CanCloseBuildEarly() || !status->ok()) child(1)->Close(state); I added this to close the build side ASAP when possible. http://gerrit.cloudera.org:8080/#/c/7223/5/be/src/exec/sort-node.cc File be/src/exec/sort-node.cc: Line 170: // Unless we are inside a subplan expecting to call Open()/GetNext() on the child Moving this wasn't strictly necessary but it seems to make sense to close it once we're done with it. http://gerrit.cloudera.org:8080/#/c/7223/5/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java: Line 103: public boolean isBlockingNode() { return false; } This was also a bug. It's a streaming node! Added a planner test with a pipeline of analytics that makes it obvious. http://gerrit.cloudera.org:8080/#/c/7223/5/fe/src/main/java/org/apache/impala/planner/PlanFragment.java File fe/src/main/java/org/apache/impala/planner/PlanFragment.java: Line 239: if (sink_ instanceof JoinBuildSink) { This was another subtle bug - we were double-counting some fragments in the parallel plans because the join build fragment was included in the parent fragment (there isn't an exchange node separating the fragments)/. -- To view, visit http://gerrit.cloudera.org:8080/7223 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I492cf5052bb27e4e335395e2a8f8a3b07248ec9d Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
