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 3: (11 comments) Thanks for your reviews. I am not sure if it is necessary to maintain the encapsulation features of the language for unit testing, so I have not made any changes to the relevant comments. If it is necessary to modify it, please reply to me. Thank you again! http://gerrit.cloudera.org:8080/#/c/19650/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19650/2//COMMIT_MSG@7 PS2, Line 7: update min/max encoded keys during bootstrappi > What about "update min/max encoded keys during bootstrapping" Done http://gerrit.cloudera.org:8080/#/c/19650/2//COMMIT_MSG@10 PS2, Line 10: tablets to bootst > tablets to bootstrap Done http://gerrit.cloudera.org:8080/#/c/19650/2//COMMIT_MSG@21 PS2, Line 21: ensure > nit: ensure Done http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tablet/cfile_set.h File src/kudu/tablet/cfile_set.h: http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tablet/cfile_set.h@150 PS2, Line 150: // Use for unit test only. > Since TEST(tserver::TabletServerTest, TestSetEncodedKeysWhenStartup) is dec I declare that friends hope that unit tests can use private functions and do not want to break the encapsulation of functions. If encapsulation can be ignored in unit testing, please let me know. http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tablet/cfile_set.h@152 PS2, Line 152: return min_encoded_key_; > friend method 'TestSetEncodedKeysWhenStartup' can access the variables 'min I declare that friends hope that unit tests can use private functions and do not want to break the encapsulation of functions. If encapsulation can be ignored in unit testing, please let me know. http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tablet/diskrowset.h File src/kudu/tablet/diskrowset.h: http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tablet/diskrowset.h@512 PS2, Line 512: base_data_ > Could firend method ' FRIEND_TEST(tserver::TabletServerTest, TestSetEncode I don't want to break the encapsulation of functions. http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc@4636 PS2, Line 4636: min_key > The two output parameters 'min_key' and 'max_key' seem not used, they are This may be a bit complicated, please allow me to explain. In the call on line 4686, these two parameters will be passed in. This is to assign values to them, so that in the call on line 4688, the assigned parameters can be passed in for verification. http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc@4653 PS2, Line 4653: auto& rowsets = c > nit: std can be omitted. How about simplify code by: Done http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc@4655 PS2, Line 4655: auto const& rs = rowsets[0]; : auto* dsr = down_cast<tablet::DiskRowSet*>(rs.get()); : ASSERT_FALSE(dsr->TEST_base_data()->TEST_min_encoded_key().empty()); : A > Now that rowsets size equals 1, how about: Done http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc@4686 PS2, Line 4686: in_key, &max_key); > nit: 'keys_in_rowset_meta' is the parameter's name in lambda above. Done http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc@4688 PS2, Line 4688: > Same. 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: 3 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Mon, 27 Mar 2023 09:35:29 +0000 Gerrit-HasComments: Yes
