Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16830 )
Change subject: [consensus] KUDU-2181 Allow sending status msgs to FAILED peers ...................................................................... Patch Set 5: (8 comments) http://gerrit.cloudera.org:8080/#/c/16830/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16830/5//COMMIT_MSG@32 PS5, Line 32: Verify the new master is promoted as VOTER > What if it's not promoted to a voter role? What's the recovery scenario? Assuming the Raft ChangeConfig went through fine, if the new master doesn't get promoted to a VOTER then leader master logs should have information about why catchup/promotion is not possible. In such a case, tablet copy part of the process will need to be repeated. I'll add that to the commit message. http://gerrit.cloudera.org:8080/#/c/16830/5/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: http://gerrit.cloudera.org:8080/#/c/16830/5/src/kudu/consensus/consensus_queue.cc@707 PS5, Line 707: peer_copy.wal_catchup_possible > Is this part is not exactly related to the changes in the context of multi- Done http://gerrit.cloudera.org:8080/#/c/16830/4/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/16830/4/src/kudu/master/dynamic_multi_master-test.cc@416 PS4, Line 416: new_master_->Shutdown(); : cluster_.reset(); : : LOG(INFO) << "Bringing up the migrated cluster"; : opts_.num_masters = orig_num_masters_ + 1; : opts_.master_rpc_addresses = master_hps; : ExternalMiniCluster migrated_cluster(opts_); : ASSERT_OK(migrated_cluster.Start()); : for (int i = 0; i < migrated_cluster.num_masters(); i++) { : ASSERT_OK(migrated_cluster.master(i)->WaitForCatalogManager()); : } : : // Verify the cluster still has the same 3 masters. : { : ListMastersResponsePB resp; : NO_FATALS(RunListMaste > Right, though this iteration of the test isn't checking any non-system tabl Done http://gerrit.cloudera.org:8080/#/c/16830/5/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/16830/5/src/kudu/master/dynamic_multi_master-test.cc@168 PS5, Line 168: Need to create around 1k tables even with lowest flush threshold and log segment size. > Might it be helpful to add dimension_label for a table to decrease the numb I tried adding a long dimension label on creating tables but that didn't help bring down the the number of tables required to trigger GC. http://gerrit.cloudera.org:8080/#/c/16830/5/src/kudu/master/dynamic_multi_master-test.cc@424 PS5, Line 424: for (int i = 0; i < migrated_cluster.num_masters(); i++) { : ASSERT_OK(migrated_cluster.master(i)->WaitForCatalogManager()); : } > nit: does it makes sense to ensure that new_master_ is now a part of master Done http://gerrit.cloudera.org:8080/#/c/16830/5/src/kudu/master/dynamic_multi_master-test.cc@438 PS5, Line 438: ASSERT_EQ(1, master.registration().rpc_addresses_size()); : HostPort actual_hp = HostPortFromPB(master.registration().rpc_addresses(0)); : ASSERT_TRUE(std::find(master_hps.begin(), master_hps.end(), actual_hp) != master_hps.end()); : > Does it make sense to check there aren't any duplicates among the registere Done http://gerrit.cloudera.org:8080/#/c/16830/5/src/kudu/master/dynamic_multi_master-test.cc@467 PS5, Line 467: master one > nit: one master Done http://gerrit.cloudera.org:8080/#/c/16830/5/src/kudu/master/dynamic_multi_master-test.cc@475 PS5, Line 475: kTableName > To harden the use case, does it make sense to try opening a table that was Done -- To view, visit http://gerrit.cloudera.org:8080/16830 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I142c1dec442ec72c38c5be9d62cdf270e441d6e3 Gerrit-Change-Number: 16830 Gerrit-PatchSet: 5 Gerrit-Owner: Bankim Bhavsar <[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-Comment-Date: Tue, 22 Dec 2020 20:50:00 +0000 Gerrit-HasComments: Yes
