Merge pull request #482 from anhanhnguyen/transfer-leadership-to-non-…

…voter Check membership before transferring leadership
rabbitmq · Dec 9, 2024 · ca694dc · ca694dc
2 parents c8dbe23 + 9b65836
commit ca694dc
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 13 deletions.
diff --git a/src/ra.erl b/src/ra.erl
@@ -1154,8 +1154,9 @@ initial_members(ServerId) ->
 initial_members(ServerId, Timeout) ->
     ra_server_proc:state_query(ServerId, initial_members, Timeout).
 
-%% @doc Transfers leadership from the leader to a follower.
+%% @doc Transfers leadership from the leader to a voter follower.
 %% Returns `already_leader' if the transfer target is already the leader.
+%% Leadership cannot be transferred to non-voters.
 %% @end
 -spec transfer_leadership(ra_server_id(), ra_server_id()) ->
     ok | already_leader | {error, term()} | {timeout, ra_server_id()}.

diff --git a/src/ra_server.erl b/src/ra_server.erl
@@ -827,17 +827,25 @@ handle_leader({transfer_leadership, Member},
            [LogId, Member]),
     {leader, State, [{reply, {error, unknown_member}}]};
 handle_leader({transfer_leadership, ServerId},
-              #{cfg := #cfg{log_id = LogId}} = State) ->
-    ?DEBUG("~ts: transfer leadership to ~w requested",
-           [LogId, ServerId]),
-    %% TODO find a timeout
-    gen_statem:cast(ServerId, try_become_leader),
-    {await_condition,
-     State#{condition =>
-            #{predicate_fun => fun transfer_leadership_condition/2,
-              timeout => #{effects => [],
-                           transition_to => leader}}},
-     [{reply, ok}]};
+              #{cfg := #cfg{log_id = LogId},
+                cluster := Cluster} = State) ->
+    case Cluster of
+        #{ServerId := #{voter_status := #{membership := Membership}}}
+          when Membership =/= voter ->
+            ?INFO("~ts: transfer leadership requested but non-voter member ~w",
+                  [LogId, ServerId]),
+            {leader, State, [{reply, {error, non_voter}}]};
+        _ ->
+            ?DEBUG("~ts: transfer leadership to ~w requested",
+                   [LogId, ServerId]),
+            %% TODO find a timeout
+            gen_statem:cast(ServerId, try_become_leader),
+            {await_condition,
+             State#{condition =>
+                        #{predicate_fun => fun transfer_leadership_condition/2,
+                          timeout => #{effects => [], transition_to => leader}}},
+             [{reply, ok}]}
+    end;
 handle_leader({register_external_log_reader, Pid}, #{log := Log0} = State) ->
     {Log, Effs} = ra_log:register_reader(Pid, Log0),
     {leader, State#{log => Log}, Effs};

diff --git a/test/ra_SUITE.erl b/test/ra_SUITE.erl
@@ -1226,7 +1226,15 @@ transfer_leadership(Config) ->
     ?assertEqual(NewLeader, NextInLine),
     ?assertEqual(already_leader, ra:transfer_leadership(NewLeader, NewLeader)),
     ?assertEqual({error, unknown_member}, ra:transfer_leadership(NewLeader, {unknown, node()})),
-    terminate_cluster(Members).
+    NonVoterMember =
+        #{id => NonVoterMemberId = {n4, node()},
+          uid => ra:new_uid(ra_lib:to_binary(Name)),
+          membership => non_voter},
+    {ok, _, _} = ra:add_member(NewLeader, NonVoterMember),
+    ok = ra:start_server(default, Name, NonVoterMember, add_machine(), Members),
+    ct:pal("Transferring leadership from ~p to ~p", [NewLeader, NonVoterMemberId]),
+    ?assertEqual({error, non_voter}, ra:transfer_leadership(NewLeader, NonVoterMemberId)),
+    terminate_cluster([NonVoterMemberId | Members]).
 
 transfer_leadership_two_node(Config) ->
     Name = ?config(test_name, Config),