Adar Dembo has posted comments on this change.

Change subject: KUDU-1377 (part 2): Add version 2 of protobuf container file
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/2815/1//COMMIT_MSG
Commit Message:

Line 13:    field (huge number) can look like a truncated file.
> What is this, journalism class? ;)
Always.


http://gerrit.cloudera.org:8080/#/c/2815/1/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:

Line 372:     RETURN_NOT_OK_PREPEND(ReadAndCompareChecksum(reader, file_size, 
&tmp_offset, { header }),
> For performance reasons? I think readahead should probably take care of the
Yeah, it would be for performance reasons, on solid state or nvram where the 
cost of the syscall may be higher than the cost of the second IO itself.

But if it's too tricky to handle well (i.e. probably need to presuppose v2, 
read 12 bytes, then throw away the last 4 bytes for v1 containers), then don't 
bother.


-- 
To view, visit http://gerrit.cloudera.org:8080/2815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I239c7b99a55a74a6a658ff418830c1f9be13eaa6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-HasComments: Yes

Reply via email to