Todd Lipcon has posted comments on this change.

Change subject: WIP: KUDU-463. Add checksumming to cfile
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6630/3/src/kudu/cfile/cfile-test.cc
File src/kudu/cfile/cfile-test.cc:

Line 819:   unique_ptr<CorruptReadableBlock> corrupt_source(
> I will look into this. I think ideally I would like to corrupt each byte in
yea, you probably need to re-open the file and clear the block cache in between


http://gerrit.cloudera.org:8080/#/c/6630/3/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

Line 363:   uint32_t data_size = ptr.size();
> ptr.size() returns uint32_t, so I kept it the same for consistency.
yea, though the style guide says to use signed quantities anywhere that you 
aren't doing bitwise ops (that way overflow will be more obvious since it will 
go to -1 instead of a very large number)


Line 392:     RETURN_NOT_OK(block_->Read(checksum_offset, sizeof(uint32_t), 
&checksum, checksum_scratch));
> I didn't want to cache the checksum since that seamed to complicate cache r
hm, possibly, but am worried that doubling the syscalls is expensive


-- 
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <[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

Reply via email to