Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19650 )
Change subject: [rowset_metadata] add min/max encoded keys update during restart ...................................................................... Patch Set 2: (5 comments) 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 declared as friend class of CFileSet, it's able to access the private members of CFileSet, why introduce new private TEST_* 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@4653 PS2, Line 4653: std::vector<std:: nit: std can be omitted. How about simplify code by: const auto& rowsets = ... http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc@4655 PS2, Line 4655: shared_ptr<tablet::RowSet> rs; : for (const shared_ptr<tablet::RowSet> &rowset : rowsets) { : rs = rowset; : } Now that rowsets size equals 1, how about: auto const& rs = rowsets[0]; http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc@4686 PS2, Line 4686: set_keys_during_restart nit: 'keys_in_rowset_meta' is the parameter's name in lambda above. http://gerrit.cloudera.org:8080/#/c/19650/2/src/kudu/tserver/tablet_server-test.cc@4688 PS2, Line 4688: set_keys_during_restart Same. -- 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: 2 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Alexey Serbin <[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 07:28:08 +0000 Gerrit-HasComments: Yes
