Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20714 )

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc@997
PS5, Line 997: ExternalMaster* ExternalMiniCluster::leader_master() {
             :   int idx = 0;
             :   Status s = GetLeaderMasterIndex(&idx);
             :   if (!s.ok()) return nullptr;
             :   return master(idx);
             : }
I guess there might be tests that used this method in its original incarnation 
in a single-master test cluster, and that usage wasn't assuming the master 
already bootstrapped and established its own Raft leadership so ConnectToMaster 
call would work.  You'd need to find those and update them correspondingly, 
otherwise there might be a lot of failures and flakiness introduced due to this 
change.


http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc@1043
PS5, Line 1043:         break;
This 'break' is to bail out of the embedded 'for' cycle, but what about exiting 
earlier from the outer 'for' cycle as well?



--
To view, visit http://gerrit.cloudera.org:8080/20714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 5
Gerrit-Owner: Ádám Bakai <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 21 Nov 2023 21:22:23 +0000
Gerrit-HasComments: Yes

Reply via email to