Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8966 )

Change subject: IMPALA-4835: Part 3: switch I/O buffers to buffer pool
......................................................................


Patch Set 24: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8966/20/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/8966/20/be/src/exec/hdfs-parquet-scanner.cc@1766
PS20, Line 1766:         bytes_to_add = 0;
> Right, but don't we reserve 3 * 8MB per column anyway? Oh, but then we'll r
Yeah we probably have enough reservation to buffer all the columns in that case 
(although not always since you have some internal fragmentation within the 
buffers). I also wanted to be reasonably efficient if we got the minimum 
reservation, not the ideal.

Oh and there's the unusual case where the row groups are not aligned to the 
HDFS blocks and the row group is actually much larger than the input split 
size. I'm not concerned about perf in that case but it's another reason things 
can get interesting.


http://gerrit.cloudera.org:8080/#/c/8966/22/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/8966/22/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1353
PS22, Line 1353:             quantizedMaxScanRangeBytes));
> I feel this should go at the beginning of this function to indicate this fu
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic09c6196b31e55b301df45cc56d0b72cfece6786
Gerrit-Change-Number: 8966
Gerrit-PatchSet: 24
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 21:57:03 +0000
Gerrit-HasComments: Yes

Reply via email to