Grant Henke has posted comments on this change. Change subject: env: Always read fully when reading files ......................................................................
Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/6758/3//COMMIT_MSG Commit Message: Line 9: In KUDU-9(see ) env_util::ReadFully was added to ensure > Missing a link here? To be honest it's not necessary; KUDU-9 is descriptive Done Line 13: Later RWFile was implimented with “read fully” behavior by default. > implemented Done Line 14: (see a15c795360e32885c00442efacd2a345f993f425) > This is the same commit hash as above; one of them is probably wrong. Done Line 17: fully read regardles, this patch moves the “read fully” behavior > Nit: regardless Done http://gerrit.cloudera.org:8080/#/c/6758/3/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: Line 376: class ShortReadRandomAccessFile : public RandomAccessFile { > Not using this anymore, can be removed? Done http://gerrit.cloudera.org:8080/#/c/6758/3/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 255: static Status DoRead(int fd, const string& filename, uint64_t offset, size_t length, > Mind fixing this? All these static functions are already in an anonymous na Done Line 273: Slice this_result(dst, r); > Not really sure what 'this_result' adds; you could just as easily s/this_re I just reverted back to the original RWFile::Read function. I agree though. -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke <granthe...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
