Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11444 )
Change subject: IMPALA-7556: part 1: handle different file systems via polymorphism ...................................................................... Patch Set 7: Code-Review+2 (1 comment) One final comment about comments. http://gerrit.cloudera.org:8080/#/c/11444/7/be/src/runtime/io/hdfs-file-reader.h File be/src/runtime/io/hdfs-file-reader.h: http://gerrit.cloudera.org:8080/#/c/11444/7/be/src/runtime/io/hdfs-file-reader.h@69 PS7, Line 69: /// Total number of bytes read remotely. This is necessary to maintain a count of : /// the number of remote scan ranges. Since IO statistics can be collected multiple : /// times for a scan range, it is necessary to keep some state about whether this : /// scan range has already been counted as remote. There is also a requirement to : /// log the number of unexpected remote bytes for a scan range. To solve both : /// requirements, maintain num_remote_bytes_ on the ScanRange and push it to the : /// reader_ once at the close of the scan range. This comment is probably hard to understand for someone who doesn't know (yet) that there is a 1to1 mapping between scan ranges and file readers. My suggestion is extend the class comments to stress this 1to1 mapping. -- To view, visit http://gerrit.cloudera.org:8080/11444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82 Gerrit-Change-Number: 11444 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 21 Sep 2018 09:42:17 +0000 Gerrit-HasComments: Yes
