Dan Hecht 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 later 
if/when we actually put some backend reservation accounting/limit in place. 
(Though we can still think of it as a reservation of an unlimited resource 
currently). Just thinking it would be nice standardize on resource terminology 
- this is effectively the min-thread-reservation. (and then we should clarify 
the thing above is a mem reservation).

Okay to defer or argue against of course.


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.


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?



--
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 17:09:36 +0000
Gerrit-HasComments: Yes

Reply via email to