Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16936 )
Change subject: [master] KUDU-2181 Raft ChangeConfig request to remove a master ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/16936/6/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/16936/6/src/kudu/master/dynamic_multi_master-test.cc@1013 PS6, Line 1013: // Prevent master leadership transfer for a deterministic test. : for (int i = 0 ; i < cluster_->num_masters(); i++) { : // Starting the cluster with following flag leads to a case sometimes : // wherein no leader gets elected leading to failure in ConnectToMaster() RPC. : // So instead set the flag after the cluster is running. : ASSERT_OK(cluster_->SetFlag(cluster_->master(i), "leader_failure_max_missed_heartbeat_periods", : "10.0")); : } > nit for here and for TestRemoveLeaderMaster scenario: does it make sense to Done http://gerrit.cloudera.org:8080/#/c/16936/6/src/kudu/master/dynamic_multi_master-test.cc@1082 PS6, Line 1082: // ASSERT_EVENTUALLY to retry in case of master leadership transfer. : ASSERT_EVENTUALLY([&] { > nit for here and elsewhere where ASSERT_EVENTUALLY added from PS5 to PS6: w RemoveMasterFromCluster() uses RunLeaderMasterRPC() which retries in case of leadership transfer, so the ASSERT_EVENTUALLY is not needed. -- To view, visit http://gerrit.cloudera.org:8080/16936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76c03b8850faef60b65f85184c0a4db7cc6759ee Gerrit-Change-Number: 16936 Gerrit-PatchSet: 6 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: Mon, 08 Feb 2021 20:48:50 +0000 Gerrit-HasComments: Yes
