Marcel Kornacker has posted comments on this change. Change subject: MT: Planner for multi-threaded execution ......................................................................
Patch Set 3: (11 comments) http://gerrit.cloudera.org:8080/#/c/2846/3/be/src/service/query-options.cc File be/src/service/query-options.cc: Line 398: "[0, 128].", value)); > should either enforce the upper bound or not mention it since it's a user-v Done http://gerrit.cloudera.org:8080/#/c/2846/3/common/thrift/DataSinks.thrift File common/thrift/DataSinks.thrift: Line 76: // only set for hash join build sinks > optional then? there isn't really a difference for lists. i was just going to leave it empty. http://gerrit.cloudera.org:8080/#/c/2846/3/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: Line 188: 44: optional i32 mt_num_cores = 1; > remove trailing ";" for consistency Done http://gerrit.cloudera.org:8080/#/c/2846/3/common/thrift/Planner.thrift File common/thrift/Planner.thrift: Line 80: // A plan: tree of plan fragments that materializes either a query result or a hash table > or a join build Done http://gerrit.cloudera.org:8080/#/c/2846/3/fe/src/main/java/com/cloudera/impala/common/TreeNode.java File fe/src/main/java/com/cloudera/impala/common/TreeNode.java: Line 40: public void clearChildren(NodeType n) { > Looks like there was a misunderstanding in last CR. My intention was to hav Done http://gerrit.cloudera.org:8080/#/c/2846/3/fe/src/main/java/com/cloudera/impala/planner/JoinBuildSink.java File fe/src/main/java/com/cloudera/impala/planner/JoinBuildSink.java: Line 51: public JoinBuildSink(JoinTableId joinTableId, PlanNode joinNode) { > seems clearer to pass in a JoinNode directly Done http://gerrit.cloudera.org:8080/#/c/2846/3/fe/src/main/java/com/cloudera/impala/planner/ParallelPlanner.java File fe/src/main/java/com/cloudera/impala/planner/ParallelPlanner.java: Line 40: * HashTableBuildNode to build another hash table) > JoinBuildSink etc. Done Line 181: PlanId parentPlanId = join.getFragment().getPlanId(); > The id assignment might be a little easier to follow if we could make the e i don't think that's really easier. you need to pass that id as an argument to createBuildPlans(), and it starts out as null. http://gerrit.cloudera.org:8080/#/c/2846/3/fe/src/main/java/com/cloudera/impala/service/Frontend.java File fe/src/main/java/com/cloudera/impala/service/Frontend.java: Line 950: if (queryCtx.request.query_options.mt_num_cores != 1) { > Also check RuntimeEnv.INSTANCE.isTestEnv(). Then this the MT option will ju Done http://gerrit.cloudera.org:8080/#/c/2846/3/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test File testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test: Line 129: | hash table id: 00 > join-table-id? in this case, we know it's a hash table, so i thought we might as well call it that, but i'm okay either way. Line 149: | tuple-ids=0 row-size=16B cardinality=1925 > In the past, checking in tests at this level of explain detail has caused f Done -- To view, visit http://gerrit.cloudera.org:8080/2846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic3c34dd3f9190a131e6f03d901b4bfcd164a5174 Gerrit-PatchSet: 3 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
