Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10256 )
Change subject: IMPALA-6957: calc thread resource requirement in planner ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/10256/6/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/10256/6/common/thrift/Frontend.thrift@416 PS6, Line 416: max_per_host_required_threads > should we just go ahead and call this a reservation? Or, we can wait for l Done. Went through and updated variables and comments to use thread_reservation/mem_reservation. There were some minor formatting changes from clang-format too. http://gerrit.cloudera.org:8080/#/c/10256/6/common/thrift/Planner.thrift File common/thrift/Planner.thrift: http://gerrit.cloudera.org:8080/#/c/10256/6/common/thrift/Planner.thrift@80 PS6, Line 80: required_threads > same question about reservation. Done http://gerrit.cloudera.org:8080/#/c/10256/6/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java: http://gerrit.cloudera.org:8080/#/c/10256/6/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@276 PS6, Line 276: .setMemEstimateBytes(0) > why do that? The ResourceProfileBuilder requires setting a mem estimate (to avoid PlanNodes implicitly leaving it at zero). This is just preserving the old behaviour (which doesn't make sense, but I've been deferring making piecemeal changes to the estimates - IMPALA-5013). -- To view, visit http://gerrit.cloudera.org:8080/10256 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10256 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 11 May 2018 19:58:45 +0000 Gerrit-HasComments: Yes
