Alex Behm 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 22: (3 comments) FE code changes lgtm http://gerrit.cloudera.org:8080/#/c/8966/20/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/20/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@177 PS20, Line 177: > As a compromise I switched to saying "scan range (i.e. hdfs split)" in the Works for me. http://gerrit.cloudera.org:8080/#/c/8966/20/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1335 PS20, Line 1335: // one IoMgr scan range per column in parallel. Scanners for row-oriented formats > Yeah it was missing the qualifier "at a time". All of them can issue multip Ahh that makes sense! 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: Preconditions.checkState(maxScanRangeBytes_ >= 0); I feel this should go at the beginning of this function to indicate this function expects the member to be set. -- 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: 22 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 01:20:57 +0000 Gerrit-HasComments: Yes
