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
