Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17528 )
Change subject: [master] AddMaster and copy sys catalog automatically ...................................................................... Patch Set 4: (12 comments) http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/dynamic_multi_master-test.cc@1569 PS3, Line 1569: for (int i = 0; i < kNumThreads; i++) { > warning: 'emplace_back' is called inside a loop; consider pre-allocating th Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc File src/kudu/master/master_runner.cc: http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@92 PS3, Line 92: using std::vector; > warning: using decl 'GetStatusRequestPB' is unused [misc-unused-using-decls Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@93 PS3, Line 93: using std::to_string; > warning: using decl 'GetStatusResponsePB' is unused [misc-unused-using-decl Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@94 PS3, Line 94: using strings::Substitute; > warning: using decl 'GenericServiceProxy' is unused [misc-unused-using-decl Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@101 PS3, Line 101: > warning: using decl 'unordered_map' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@145 PS3, Line 145: the local hostport respectively. Sets 'needs_add' to whether 'local_uuid' : // should be added to the Raft config. These out-parameters should only be : // used if OK is returned and 'needs_re > Could you clarify the purpose of "local_hp" and "has_errors" as out paramet Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@197 PS3, Line 197: master_addr.ToString(), s.ToString()); > It'd be good to supply the system catalog tablet id in the request instead Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@230 PS3, Line 230: > const auto& We can't move() this if it's const. http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@233 PS3, Line 233: } > Might be worth printing the symmetric set difference to help debug. Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@327 PS3, Line 327: SET_FLAGS_DEFAULT)); : // A multi-node Master leader should not evict failed Master followers : // > In addition to checking for size, lets also check that opts.master_addresse Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@334 PS3, Line 334: // > Could the part below of adding master/deleting catalog/copying catalog move Done http://gerrit.cloudera.org:8080/#/c/17528/3/src/kudu/master/master_runner.cc@371 PS3, Line 371: > It'd be good to add a log line mentioning the local sys catalog is being de Done -- To view, visit http://gerrit.cloudera.org:8080/17528 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic7635723f30317a45799ad7b9c9b725eafbd735b Gerrit-Change-Number: 17528 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 07 Jul 2021 17:52:03 +0000 Gerrit-HasComments: Yes
