Todd Lipcon has posted comments on this change. Change subject: WIP: KUDU-463. Add checksumming to cfile ......................................................................
Patch Set 6: (2 comments) Just did a quick look at the approach using readv, it seems like a good direction to me. I didn't review the new header/footer checksum stuff yet. It may be worth checking for Adar's opinion on the API in Env, he tends to have opinions on that stuff. The current one doesn't seem super idiomatic to me but also don't really have a better suggestion :) 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 PS6, Line 472: && || -- 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 <granthe...@gmail.com> Gerrit-Reviewer: Grant Henke <granthe...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
