Grant Henke has posted comments on this change. Change subject: WIP: KUDU-463. Add checksumming to cfile ......................................................................
Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/6630/6/src/kudu/cfile/cfile_reader.cc File src/kudu/cfile/cfile_reader.cc: Line 446: data_size -= checksum_size; > careful of underflow Done Line 472: if (block.size() != data_size && checksum.size() != checksum_size) { > || Done http://gerrit.cloudera.org:8080/#/c/6630/6/src/kudu/util/env.h File src/kudu/util/env.h: Line 21: #include <sys/uio.h> > Would prefer not to leak this sort of platform-specific thing outside of en I agree. This was just a rough patch to see if the approach and use of preadv made sense before polishing. See my comment below as well. Line 390: virtual Status ReadV(uint64_t offset, Slice** results, > Would std::vector<Slice>* be more intuitive for 'results'? That is, ReadV() I agree that a vector with all the combined parameters would be better in the end. Something that has slice, scratch, and length so we don't need to expose iovec. I planned to do this, I just posted this super rough version to see if the approach and use of preadv made sense before spending the time polishing, testing, etc. -- To view, visit http://gerrit.cloudera.org:8080/6630 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
