Pooja Nilangekar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11001 )

Change subject: IMPALA-7234: Improve memory estimates produced by the Planner
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11001/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11001/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1364
PS6, Line 1364:     if (fileFormats_.contains(HdfsFileFormat.PARQUET)
              :         || fileFormats_.contains(HdfsFileFormat.ORC)) {
              :       columnReservations = computeMinColumnMemReservati
> I think it would be clearer if we iterated over the formats, computed the p
Done


http://gerrit.cloudera.org:8080/#/c/11001/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1370
PS6, Line 1370: ) {
              :       int partitionScanRange = 0
> This is a bit misleading since this calculation is purely an estimate and d
Done


http://gerrit.cloudera.org:8080/#/c/11001/6/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
File fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java:

http://gerrit.cloudera.org:8080/#/c/11001/6/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java@130
PS6, Line 130: itions, we want to be
             :       // conservative and make a high
> We're not really reserving anything based on this estimate for now - maybe
Done


http://gerrit.cloudera.org:8080/#/c/11001/6/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java@137
PS6, Line 137:     return 100L * 1024L;
> Yeah these estimates are pretty bogus :). We will revisit them at some poin
Ack



--
To view, visit http://gerrit.cloudera.org:8080/11001
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0666ae3d45fbd8615d3fa9a8626ebd29cf94fb4b
Gerrit-Change-Number: 11001
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Pooja Nilangekar <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Mon, 30 Jul 2018 21:47:36 +0000
Gerrit-HasComments: Yes

Reply via email to