Sahil Takiar 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) 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, t Yeah, that is a good observation. Looking through the code you are right. For readFully it throws an EOFException if you try to read past the end of the file, but for read it just returns -1. Right now libhdfs doesn't translate Java EOFException's to a specific error code so we just get back the generic errno = EINTERNAL. Although I think it might be fine if this fails on EOF. I think the only way this could happen is if (1) the file is overwritten, or (2) there is some internal error and we don't calculate the length of the file properly. Either way I would think it is okay if Impala fails, since overwriting a file while it is being read is probably undefined behavior? Even if it worked previously. 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): ACK, will fix this. -- 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: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 06 Nov 2019 17:53:15 +0000 Gerrit-HasComments: Yes
