Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14384 )
Change subject: IMPALA-8999: make union scheduling work with mt_dop ...................................................................... Patch Set 12: (4 comments) http://gerrit.cloudera.org:8080/#/c/14384/12/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: http://gerrit.cloudera.org:8080/#/c/14384/12/be/src/scheduling/scheduler.cc@338 PS12, Line 338: for the scan range nit: remove? http://gerrit.cloudera.org:8080/#/c/14384/12/be/src/scheduling/scheduler.cc@344 PS12, Line 344: Note that this takes into account all of the input fragments, : // not just the leftmost because we expect unions to be symmetrical for purposes of : // planning, unlike joins. nit: how about: "Note that step 1 is modified to run on fragments with union nodes, by considering all input fragments and not just the leftmost because we expect unions to be symmetrical for purposes of planning, unlike joins." http://gerrit.cloudera.org:8080/#/c/14384/12/be/src/scheduling/scheduler.cc@392 PS12, Line 392: vector<TNetworkAddress> scan_hosts; nit: maybe add a Dcheck(scan_node_ids.size() == 1 || has_union) so that we make sure this methods gets another look whenever this changes. http://gerrit.cloudera.org:8080/#/c/14384/11/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-scheduling.test File testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-scheduling.test: http://gerrit.cloudera.org:8080/#/c/14384/11/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-scheduling.test@133 PS11, Line 133: ==== > I think this is covered by the tests on l32 and l52 - alltypes has 24 files Got it, thanks for the explanation. -- To view, visit http://gerrit.cloudera.org:8080/14384 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d2e9c86b530da3053e49d42b837dca0b1348ff2 Gerrit-Change-Number: 14384 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 21 Oct 2019 20:42:40 +0000 Gerrit-HasComments: Yes
