Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18572 )
Change subject: [Tools] Rebuild master according to part of tables not all tables ...................................................................... Patch Set 22: (11 comments) > Patch Set 21: > > (11 comments) http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h File src/kudu/master/sys_catalog.h: http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h@104 PS21, Line 104: class TableInfoLoader : public TableVisitor { > Seems similar to class TestTableLoader in src/kudu/master/sys_catalog-test. Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.h@415 PS21, Line 415: > I don't think it's necessary to make it as a member, isn't it? Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.cc File src/kudu/master/sys_catalog.cc: http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/master/sys_catalog.cc@1165 PS21, Line 1165: const SysTablesEntryPB& metadata) { > nit: alignment Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3143 PS21, Line 3143: > nit: alignment Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3157 PS21, Line 3157: sterVerifier cv1(cluster_.get > Have you miss this test case? Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3177 PS21, Line 3177: fo> > Should be 'default.foo1'? And it's better to use kTable1 instead. Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3187 PS21, Line 3187: ASSERT_NE(nullptr, table_info); > should break? Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/kudu-admin-test.cc@3191 PS21, Line 3191: vector<scoped_refptr<TabletInfo>> tablets; > You can destroy 'master' then you don't have to close_fs_manager_for_test m Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc File src/kudu/tools/master_rebuilder.cc: http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc@411 PS21, Line 411: > Naming it as <value>_by_<key> is a good practice in Kudu, e.g. std::unorder Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/master_rebuilder.cc@425 PS21, Line 425: : RETURN_NOT_OK(DoWrite(&sys_catalog, tab > Could you plz use ContainsKey in src/kudu/gutil/map-util.h to simplify code Done http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/tool_action_master.cc File src/kudu/tools/tool_action_master.cc: http://gerrit.cloudera.org:8080/#/c/18572/21/src/kudu/tools/tool_action_master.cc@931 PS21, Line 931: " - The parameter 'tables' can be used to rebuild part of tables, when these tables " > Add a blank space before '-' Done -- To view, visit http://gerrit.cloudera.org:8080/18572 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45d812c8a16228ac937603872161969eb05136eb Gerrit-Change-Number: 18572 Gerrit-PatchSet: 22 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 13 Jul 2022 08:54:54 +0000 Gerrit-HasComments: Yes
