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 3: Code-Review+1 (1 comment) I think this change makes sense. I'm willing to bump this to +2. Take a look at Tim's comment about mentioning ABFS perf. 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) { > Yeah, that is a good observation. Looking through the code you are right. F Thinking about it another way: If someone is overwriting files and they overwrite with something larger, then I think we would not read the whole file. That would cause its own set of problems. So, I'm thinking that this is undefined behavior and this behavior is not something users should be relying on. -- 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: 3 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: Tue, 12 Nov 2019 21:06:23 +0000 Gerrit-HasComments: Yes