KeDeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/19650 )
Change subject: [rowset_metadata] update min/max encoded keys during bootstrapping ...................................................................... Patch Set 7: (12 comments) Thanks for your reviews. http://gerrit.cloudera.org:8080/#/c/19650/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19650/5//COMMIT_MSG@10 PS5, Line 10: initialize t > initialize Done http://gerrit.cloudera.org:8080/#/c/19650/5//COMMIT_MSG@21 PS5, Line 21: test > test case Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tablet/cfile_set.cc File src/kudu/tablet/cfile_set.cc: http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tablet/cfile_set.cc@174 PS5, Line 174: // Verify the loaded keys are valid. : if (Slice(min_encoded_key_) > max_encoded_key_) { : return Status::Corruption(Substitute("Min key $0 > max key $1", : KUDU_REDACT(Slice(min_encoded_key_).ToDebugString()), : > Does it make sense to do so after verifying the validity of the keys? In o Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4615 PS5, Line 4615: SetEncodedKeysWhenStartingUp) > nit: consider renaming into Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4618 PS5, Line 4618: when restarting. > when restarting Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4628 PS5, Line 4628: key do not exist > keys do not exist Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4630 PS5, Line 4630: RowSetDataPB* rowset_pb = superblock_pb.mutable_rowsets(rowset_no); > nit: would it be enough to access just constant rowsets(rowset_no) field wi I just want confirm that the keys in the metadata file does not record the min/max key before restarting and after restarting without setting --rowset_metadata_store_keys. Based on your suggestion, I can add metadata comparison for the metadata of membrowset. http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4634 PS5, Line 4634: > Used Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4669 PS5, Line 4669: ASSERT_FALSE(dsr->base_data_->max_encoded_key_.empty()); > ditto for considering using accessor for immutable field instead Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4677 PS5, Line 4677: > either 'There is no encoded key without ...' or 'The encoded key doesn't ex Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4685 PS5, Line 4685: b->has_min_e > nit: the encoded keys Done http://gerrit.cloudera.org:8080/#/c/19650/5/src/kudu/tserver/tablet_server-test.cc@4685 PS5, Line 4685: ed_key()); > nit: either '... we set --rowset_metadata_store_keys ...' or '... we set th Done -- To view, visit http://gerrit.cloudera.org:8080/19650 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b7ed75bdf7a2ff0e16c2670f1a6f9819ee8e8d3 Gerrit-Change-Number: 19650 Gerrit-PatchSet: 7 Gerrit-Owner: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Comment-Date: Tue, 28 Mar 2023 08:10:25 +0000 Gerrit-HasComments: Yes
