Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14635 )
Change subject: IMPALA-8525: preads should use hdfsPreadFully rather than hdfsPread ...................................................................... Patch Set 1: (2 comments) First pass http://gerrit.cloudera.org:8080/#/c/14635/1/be/src/runtime/io/hdfs-file-reader.cc File be/src/runtime/io/hdfs-file-reader.cc: http://gerrit.cloudera.org:8080/#/c/14635/1/be/src/runtime/io/hdfs-file-reader.cc@225 PS1, Line 225: if (hdfsPreadFully( : hdfs_fs_, hdfs_file, position_in_file, buffer, bytes_to_read) == -1) { I'm thinking through the error case. If we read past the end of the file, this will result in an error. In the old world, we would read to EOF and stop without an error. Even if we tried to read more (i.e. issue an IO at the end of the file), I think it would EOF (return 0 bytes read) and we would give up without an error. It's a corner case. This could happen if files get overwritten. http://gerrit.cloudera.org:8080/#/c/14635/1/be/src/runtime/io/scan-range.cc File be/src/runtime/io/scan-range.cc: http://gerrit.cloudera.org:8080/#/c/14635/1/be/src/runtime/io/scan-range.cc@a31 PS1, Line 31: : : : : : : : We have a graveyard for removed flags (complete with snazzy ASCII art): https://github.com/apache/impala/blob/master/be/src/common/global-flags.cc#L289-L307 This allows Impala to continue to start up if anyone specifies these dead flags. -- To view, visit http://gerrit.cloudera.org:8080/14635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I29ea34897096bc790abdeb98073a47f1c4c10feb Gerrit-Change-Number: 14635 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 05 Nov 2019 18:46:24 +0000 Gerrit-HasComments: Yes
