Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/20690 )
Change subject: [cfile] clean up on IndexBlock{Builder,Iterator,Reader}
......................................................................
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index-test.cc
File src/kudu/cfile/index-test.cc:
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index-test.cc@348
PS7, Line 348: TEST(TestIndexBlock, EmptyBlock) {
> nit: Add a one liner about what this test does
Done
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index-test.cc@404
PS7, Line 404: 2048
> How is this different from 8 keys iterations above? Testing boundary condit
It differs in the number of keys added into a block. All the sanity checks in
the parser are based on min/max estimates, and there might be variation in the
size of the encoded fields because of varint encoding, where bigger values
consume more space when serialized.
I added a scope-wide comment correspondingly.
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index_block.cc
File src/kudu/cfile/index_block.cc:
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index_block.cc@268
PS7, Line 268: if (PREDICT_FALSE(data_.data() + offset_in_block >=
key_offsets_)) {
: return Status::Corruption(Substitute(
: "$0: invalid block offset at index $1", offset_in_block,
idx_in_block));
: }
> Is it possible to cover this and other uncovered error scenarios in the uni
Yes, but I'd rather do that in a separate patch. My plan for this patch was to
harden the parser: I did so and added coverage for all the error paths in
IndexBlockReader::Parse().
--
To view, visit http://gerrit.cloudera.org:8080/20690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If83dd132b577a481a2ddaa09e2657639f8b92c7d
Gerrit-Change-Number: 20690
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 16 Nov 2023 02:41:24 +0000
Gerrit-HasComments: Yes
