From 22b272c61ef42be0a19589ea25227897f180705b Mon Sep 17 00:00:00 2001 From: Alex Valiushko Date: Sat, 6 May 2023 15:15:28 -0700 Subject: [PATCH 1/8] Newly added followers do not participate in quorum until they catch up with the log An opt-in ability of a cluster to ignore newly joined member until it catches up with the log: New = #{id => Id, ini_non_voter => ra:new_nvid()}, {ok, _, _} = ra:add_member(ServerRef, New), ok = ra:start_server(default, ClusterName, New, add_machine(), [ServerRef]), Voter status is stored in the cluster map of the server state and is part of every $ra_cluster_update. Additionally, nodes store their own status at the top level for ease of matching. Nodes also store their own satus in ra_state ETS table (breaking change), and present in overview. On every #append_entries_reply leader may choose to promote non-voter by issuing new `$ra_join` with desired voter status. Currently, only one promotion condition is implemented `{nonvoter, #{target := ra_index()}`. Non-voter will be promoted when it reaches the leaders log index at the time of joining. --- src/ra.erl | 39 ++++-- src/ra.hrl | 20 +++ src/ra_directory.erl | 10 +- src/ra_log.erl | 1 + src/ra_server.erl | 218 ++++++++++++++++++++++++++---- src/ra_server_proc.erl | 23 +++- test/coordination_SUITE.erl | 115 ++++++++++++++++ test/ra_2_SUITE.erl | 44 ++++++- test/ra_SUITE.erl | 96 +++++++++++++- test/ra_log_snapshot_SUITE.erl | 1 + test/ra_server_SUITE.erl | 234 ++++++++++++++++++++++++++++++--- 11 files changed, 735 insertions(+), 66 deletions(-) diff --git a/src/ra.erl b/src/ra.erl index 286bb46b..ff0ba778 100644 --- a/src/ra.erl +++ b/src/ra.erl @@ -71,6 +71,7 @@ overview/1, %% helpers new_uid/1, + new_nvid/0, %% rebalancing transfer_leadership/2, %% auxiliary commands @@ -455,7 +456,7 @@ start_cluster(System, [#{cluster_name := ClusterName} | _] = ServerConfigs, %% @doc Starts a new distributed ra cluster. %% @param ClusterName the name of the cluster. -%% @param ServerId the ra_server_id() of the server +%% @param ServerId the ra_server_id() of the server, or a map with server id and settings. %% @param Machine The {@link ra_machine:machine/0} configuration. %% @param ServerIds a list of initial (seed) server configurations %% @returns @@ -470,19 +471,20 @@ start_cluster(System, [#{cluster_name := ClusterName} | _] = ServerConfigs, %% forcefully deleted. %% @see start_server/1 %% @end --spec start_server(atom(), ra_cluster_name(), ra_server_id(), +-spec start_server(atom(), ra_cluster_name(), ra_server_id() | ra_new_server(), ra_server:machine_conf(), [ra_server_id()]) -> ok | {error, term()}. -start_server(System, ClusterName, {_, _} = ServerId, Machine, ServerIds) +start_server(System, ClusterName, {_, _} = ServerId, Machine, ServerIds) -> + start_server(System, ClusterName, #{id => ServerId}, Machine, ServerIds); +start_server(System, ClusterName, #{id := {_, _}} = Conf0, Machine, ServerIds) when is_atom(System) -> UId = new_uid(ra_lib:to_binary(ClusterName)), Conf = #{cluster_name => ClusterName, - id => ServerId, uid => UId, initial_members => ServerIds, log_init_args => #{uid => UId}, machine => Machine}, - start_server(System, Conf). + start_server(System, maps:merge(Conf0, Conf)). %% @doc Starts a ra server in the default system %% @param Conf a ra_server_config() configuration map. @@ -558,9 +560,10 @@ delete_cluster(ServerIds, Timeout) -> %% affect said cluster's availability characteristics (by increasing quorum node count). %% %% @param ServerLoc the ra server or servers to try to send the command to -%% @param ServerId the ra server id of the new server. +%% @param ServerId the ra server id of the new server, or a map with server id and settings. %% @end --spec add_member(ra_server_id() | [ra_server_id()], ra_server_id()) -> +-spec add_member(ra_server_id() | [ra_server_id()], + ra_server_id() | ra_new_server()) -> ra_cmd_ret() | {error, already_member} | {error, cluster_change_not_permitted}. @@ -571,7 +574,8 @@ add_member(ServerLoc, ServerId) -> %% @see add_member/2 %% @end -spec add_member(ra_server_id() | [ra_server_id()], - ra_server_id(), timeout()) -> + ra_server_id() | ra_new_server(), + timeout()) -> ra_cmd_ret() | {error, already_member} | {error, cluster_change_not_permitted}. @@ -580,7 +584,6 @@ add_member(ServerLoc, ServerId, Timeout) -> {'$ra_join', ServerId, after_log_append}, Timeout). - %% @doc Removes a server from the cluster's membership configuration. %% This function returns after appending a cluster membership change %% command to the log. @@ -716,6 +719,13 @@ new_uid(Source) when is_binary(Source) -> Prefix = ra_lib:derive_safe_string(Source, 6), ra_lib:make_uid(string:uppercase(Prefix)). +%% @doc generates a random uid using timestamp for the first +%% 6 characters. +%% @end +new_nvid() -> + Millis = erlang:system_time(millisecond), + Prefix = base64:encode(<>), + new_uid(Prefix). %% @doc Returns a map of overview data of the default Ra system on the current Erlang %% node. @@ -1132,13 +1142,16 @@ key_metrics({Name, N} = ServerId) when N == node() -> end, case whereis(Name) of undefined -> - Counters#{state => noproc}; + Counters#{state => noproc, + voter_status => noproc}; _ -> case ets:lookup(ra_state, Name) of [] -> - Counters#{state => unknown}; - [{_, State}] -> - Counters#{state => State} + Counters#{state => unknown, + voter_status => unknown}; + [{_, State, Voter}] -> + Counters#{state => State, + voter_status => Voter} end end; key_metrics({_, N} = ServerId) -> diff --git a/src/ra.hrl b/src/ra.hrl index 08d5c5b3..e6a560b6 100644 --- a/src/ra.hrl +++ b/src/ra.hrl @@ -33,23 +33,42 @@ %% used for on disk resources and local name to pid mapping -type ra_uid() :: binary(). +%% Transient ID that uniquely identifies any new non-voter. +-type ra_nvid() :: binary(). + %% Identifies a Ra server (node) in a Ra cluster. %% %% Ra servers need to be registered stable names (names that are reachable %% after node restart). Pids are not stable in this sense. -type ra_server_id() :: {Name :: atom(), Node :: node()}. +%% Specifies server configuration for a new cluster member. +%% Both `ra:add_member` and `ra:start_server` must be called with the same value. +-type ra_new_server() :: #{id := ra_server_id(), + + %% If set, server will start as non-voter until later promoted by the + %% leader. + init_non_voter => ra_nvid()}. + -type ra_peer_status() :: normal | {sending_snapshot, pid()} | suspended | disconnected. +-type ra_voter_status() :: {voter, ra_voter_state()} | + {nonvoter, ra_voter_state()}. + +-type ra_voter_state() :: #{nvid => ra_nvid(), + target => ra_index()}. + -type ra_peer_state() :: #{next_index := non_neg_integer(), match_index := non_neg_integer(), query_index := non_neg_integer(), % the commit index last sent % used for evaluating pipeline status commit_index_sent := non_neg_integer(), + %% whether the peer is part of the consensus + voter_status := ra_voter_status(), %% indicates that a snapshot is being sent %% to the peer status := ra_peer_status()}. @@ -139,6 +158,7 @@ -type snapshot_meta() :: #{index := ra_index(), term := ra_term(), cluster := ra_cluster_servers(), + cluster_state => ra_cluster(), %% TODO replace `cluster` machine_version := ra_machine:version()}. -record(install_snapshot_rpc, diff --git a/src/ra_directory.erl b/src/ra_directory.erl index 0a393fea..1e42f88b 100644 --- a/src/ra_directory.erl +++ b/src/ra_directory.erl @@ -175,14 +175,20 @@ overview(System) when is_atom(System) -> #{directory := Tbl, directory_rev := _TblRev} = get_names(System), Dir = ets:tab2list(Tbl), - States = maps:from_list(ets:tab2list(ra_state)), + Rows = lists:map(fun({K, S, V}) -> + {K, {S, V}} + end, + ets:tab2list(ra_state)), + States = maps:from_list(Rows), Snaps = maps:from_list(ets:tab2list(ra_log_snapshot_state)), lists:foldl(fun ({UId, Pid, Parent, ServerName, ClusterName}, Acc) -> + {S, V} = maps:get(ServerName, States, {undefined, undefined}), Acc#{ServerName => #{uid => UId, pid => Pid, parent => Parent, - state => maps:get(ServerName, States, undefined), + state => S, + voter_status => V, cluster_name => ClusterName, snapshot_state => maps:get(UId, Snaps, undefined)}} diff --git a/src/ra_log.erl b/src/ra_log.erl index d8371396..5bd897e8 100644 --- a/src/ra_log.erl +++ b/src/ra_log.erl @@ -644,6 +644,7 @@ update_release_cursor0(Idx, Cluster, MacVersion, MacState, end, Meta = #{index => Idx, cluster => ClusterServerIds, + cluster_state => Cluster, machine_version => MacVersion}, % The release cursor index is the last entry _not_ contributing % to the current state. I.e. the last entry that can be discarded. diff --git a/src/ra_server.erl b/src/ra_server.erl index 4a67bff5..760581a5 100644 --- a/src/ra_server.erl +++ b/src/ra_server.erl @@ -55,6 +55,7 @@ terminate/2, log_fold/3, log_read/2, + voter_status/1, recover/1 ]). @@ -72,6 +73,7 @@ log := term(), voted_for => 'maybe'(ra_server_id()), % persistent votes => non_neg_integer(), + voter_status => ra_voter_status(), commit_index := ra_index(), last_applied := ra_index(), persisted_last_applied => ra_index(), @@ -195,6 +197,7 @@ max_pipeline_count => non_neg_integer(), ra_event_formatter => {module(), atom(), [term()]}, counter => counters:counters_ref(), + init_non_voter => ra_nvid(), system_config => ra_system:config()}. -type mutable_config() :: #{cluster_name => ra_cluster_name(), @@ -297,13 +300,17 @@ init(#{id := Id, case ra_log:recover_snapshot(Log0) of undefined -> InitialMachineState = ra_machine:init(Machine, Name), - {0, make_cluster(Id, InitialNodes), + Clu = make_cluster(Id, InitialNodes, + #{voter_status => init_voter_status(Config)}), + {0, Clu, 0, InitialMachineState, {0, 0}}; {#{index := Idx, term := Term, - cluster := ClusterNodes, - machine_version := MacVersion}, MacSt} -> - Clu = make_cluster(Id, ClusterNodes), + cluster := ClusterNodes0, + machine_version := MacVersion} = Snapshot, MacSt} -> + ClusterNodes = maps:get(cluster_state, Snapshot, ClusterNodes0), + Clu = make_cluster(Id, ClusterNodes, + #{voter_status => init_voter_status(Config)}), %% the snapshot is the last index before the first index %% TODO: should this be Idx + 1? {Idx + 1, Clu, MacVersion, MacSt, {Idx, Term}} @@ -338,6 +345,7 @@ init(#{id := Id, cluster_change_permitted => false, cluster_index_term => SnapshotIndexTerm, voted_for => VotedFor, + voter_status => voter_status(Id, Cluster0), commit_index => CommitIndex, %% set this to the first index so that we can apply all entries %% up to the commit index during recovery @@ -400,8 +408,8 @@ handle_leader({PeerId, #append_entries_reply{term = Term, success = true, Peer = Peer0#{match_index => max(MI, LastIdx), next_index => max(NI, NextIdx)}, State1 = put_peer(PeerId, Peer, State0), - {State2, Effects0} = evaluate_quorum(State1, []), - + Effects00 = maybe_promote_peer(PeerId, State1, []), + {State2, Effects0} = evaluate_quorum(State1, Effects00), {State, Effects1} = process_pending_consistent_queries(State2, Effects0), Effects = [{next_event, info, pipeline_rpcs} | Effects1], @@ -782,7 +790,7 @@ handle_candidate(#request_vote_result{term = Term, vote_granted = true}, NewVotes = Votes + 1, ?DEBUG("~ts: vote granted for term ~b votes ~b", [LogId, Term, NewVotes]), - case trunc(maps:size(Nodes) / 2) + 1 of + case required_quorum(Nodes) of NewVotes -> {State1, Effects} = make_all_rpcs(initialise_peers(State0)), Noop = {noop, #{ts => erlang:system_time(millisecond)}, @@ -928,7 +936,7 @@ handle_pre_vote(#pre_vote_result{term = Term, vote_granted = true, [LogId, Token, Term, Votes + 1]), NewVotes = Votes + 1, State = update_term(Term, State0), - case trunc(maps:size(Nodes) / 2) + 1 of + case required_quorum(Nodes) of NewVotes -> call_for_election(candidate, State); _ -> @@ -1110,8 +1118,18 @@ handle_follower({ra_log_event, Evt}, State = #{log := Log0}) -> % simply forward all other events to ra_log {Log, Effects} = ra_log:handle_event(Evt, Log0), {follower, State#{log => Log}, Effects}; +handle_follower(#pre_vote_rpc{}, + #{cfg := #cfg{log_id = LogId}, voter_status := {nonvoter, _} = Voter} = State) -> + ?DEBUG("~s: follower ignored pre_vote_rpc, non-voter: ~p0", + [LogId, Voter]), + {follower, State, []}; handle_follower(#pre_vote_rpc{} = PreVote, State) -> process_pre_vote(follower, PreVote, State); +handle_follower(#request_vote_rpc{}, + #{cfg := #cfg{log_id = LogId}, voter_status := {nonvoter, _} = Voter} = State) -> + ?DEBUG("~s: follower ignored request_vote_rpc, non-voter: ~p0", + [LogId, Voter]), + {follower, State, []}; handle_follower(#request_vote_rpc{candidate_id = Cand, term = Term}, #{current_term := Term, voted_for := VotedFor, cfg := #cfg{log_id = LogId}} = State) @@ -1208,6 +1226,12 @@ handle_follower(#append_entries_reply{}, State) -> %% handle to avoid logging as unhandled %% could receive a lot of these shortly after standing down as leader {follower, State, []}; +handle_follower(election_timeout, + #{cfg := #cfg{log_id = LogId}, + voter_status := {nonvoter, _} = Voter} = State) -> + ?DEBUG("~s: follower ignored election_timeout, non-voter: ~p0", + [LogId, Voter]), + {follower, State, []}; handle_follower(election_timeout, State) -> call_for_election(pre_vote, State); handle_follower(try_become_leader, State) -> @@ -1267,13 +1291,15 @@ handle_receive_snapshot(#install_snapshot_rpc{term = Term, Cfg0 end, - {#{cluster := ClusterIds}, MacState} = ra_log:recover_snapshot(Log), + {#{cluster := ClusterIds0} = Snapshot, MacState} = ra_log:recover_snapshot(Log), + ClusterIds = maps:get(cluster_state, Snapshot, ClusterIds0), State = update_term(Term, State0#{cfg => Cfg, log => Log, commit_index => SnapIndex, last_applied => SnapIndex, - cluster => make_cluster(Id, ClusterIds), + cluster => make_cluster(Id, ClusterIds, + #{voter_status => init_voter_status(State0)}), machine_state => MacState}), %% it was the last snapshot chunk so we can revert back to %% follower status @@ -1380,7 +1406,8 @@ overview(#{cfg := #cfg{effective_machine_module = MacMod} = Cfg, cluster_index_term, query_index ], State), - O = maps:merge(O0, cfg_to_map(Cfg)), + O1 = O0#{voter_status => voter_status(State)}, + O = maps:merge(O1, cfg_to_map(Cfg)), LogOverview = ra_log:overview(Log), MacOverview = ra_machine:overview(MacMod, MacState), O#{log => LogOverview, @@ -2092,6 +2119,7 @@ new_peer() -> match_index => 0, commit_index_sent => 0, query_index => 0, + voter_status => {voter, #{}}, status => normal}. new_peer_with(Map) -> @@ -2212,25 +2240,38 @@ fetch_term(Idx, #{log := Log0} = State) -> {Term, State#{log => Log}} end. -make_cluster(Self, Nodes) -> - case lists:foldl(fun(N, Acc) -> - Acc#{N => new_peer()} - end, #{}, Nodes) of +make_cluster(Self, Nodes0, DefaultSelf) when is_list(Nodes0) -> + Nodes = lists:foldl(fun(N, Acc) -> + Acc#{N => new_peer()} + end, #{}, Nodes0), + make_cluster0(Self, Nodes, DefaultSelf); +make_cluster(Self, Nodes0, DefaultSelf) when is_map(Nodes0) -> + Nodes = maps:map(fun(_, V0) -> + V1 = maps:with([voter_status], V0), + maps:merge(new_peer(), V1) + end, Nodes0), + make_cluster0(Self, Nodes, DefaultSelf). + +make_cluster0(Self, Nodes, DefaultSelf) -> + case Nodes of #{Self := _} = Cluster -> % current server is already in cluster - do nothing Cluster; Cluster -> % add current server to cluster - Cluster#{Self => new_peer()} + Cluster#{Self => maps:merge(new_peer(), DefaultSelf)} end. -initialise_peers(State = #{log := Log, cluster := Cluster0}) -> - PeerIds = peer_ids(State), +initialise_peers(State = #{cfg := #cfg{id = Id}, log := Log, cluster := Cluster0}) -> NextIdx = ra_log:next_index(Log), - Cluster = lists:foldl(fun(PeerId, Acc) -> - Acc#{PeerId => - new_peer_with(#{next_index => NextIdx})} - end, Cluster0, PeerIds), + Cluster = maps:map(fun (PeerId, Self) when PeerId =:= Id -> + Self; + (_, Peer) -> + %% if key not present we assume `voter' status + Voter = maps:get(voter_status, Peer, {voter, #{}}), + new_peer_with(#{next_index => NextIdx, + voter_status => Voter}) + end, Cluster0), State#{cluster => Cluster}. apply_to(ApplyTo, State, Effs) -> @@ -2326,6 +2367,7 @@ apply_with({Idx, Term, {'$ra_cluster_change', CmdMeta, NewCluster, ReplyType}}, [log_id(State0), maps:keys(NewCluster)]), %% we are recovering and should apply the cluster change State0#{cluster => NewCluster, + voter_status => maybe_promote_self(NewCluster, State0), cluster_change_permitted => true, cluster_index_term => {Idx, Term}}; _ -> @@ -2458,16 +2500,40 @@ append_log_leader({CmdTag, _, _, _}, when CmdTag == '$ra_join' orelse CmdTag == '$ra_leave' -> {not_appended, cluster_change_not_permitted, State}; +append_log_leader({'$ra_join', From, + #{id := JoiningNode, voter_status := Voter0}, + ReplyMode}, + State = #{cluster := OldCluster}) -> + case ensure_promotion_target(Voter0, State) of + {error, Reason} -> + {not_appended, Reason, State}; + {ok, Voter} -> + case OldCluster of + #{JoiningNode := #{voter_status := Voter}} -> + already_member(State); + #{JoiningNode := Peer} -> + % Update member status. + Cluster = OldCluster#{JoiningNode => Peer#{voter_status => Voter}}, + append_cluster_change(Cluster, From, ReplyMode, State); + _ -> + % Insert new member. + Cluster = OldCluster#{JoiningNode => new_peer_with(#{voter_status => Voter})}, + append_cluster_change(Cluster, From, ReplyMode, State) + end + end; +append_log_leader({'$ra_join', From, #{id := JoiningNode} = Spec, ReplyMode}, + State) -> + append_log_leader({'$ra_join', From, + #{id => JoiningNode, voter_status => init_voter_status(Spec)}, + ReplyMode}, State); append_log_leader({'$ra_join', From, JoiningNode, ReplyMode}, State = #{cluster := OldCluster}) -> + % Legacy $ra_join, join as voter if no such member in the cluster. case OldCluster of #{JoiningNode := _} -> - % already a member do nothing - % TODO: reply? If we don't reply the caller may block until timeout - {not_appended, already_member, State}; + already_member(State); _ -> - Cluster = OldCluster#{JoiningNode => new_peer()}, - append_cluster_change(Cluster, From, ReplyMode, State) + append_log_leader({'$ra_join', From, #{id => JoiningNode}, ReplyMode}, State) end; append_log_leader({'$ra_leave', From, LeavingServer, ReplyMode}, State = #{cfg := #cfg{log_id = LogId}, @@ -2509,6 +2575,7 @@ pre_append_log_follower({Idx, Term, Cmd} = Entry, pre_append_log_follower({Idx, Term, {'$ra_cluster_change', _, Cluster, _}}, State) -> State#{cluster => Cluster, + voter_status => maybe_promote_self(Cluster, State), cluster_index_term => {Idx, Term}}; pre_append_log_follower(_, State) -> State. @@ -2587,6 +2654,8 @@ query_indexes(#{cfg := #cfg{id = Id}, query_index := QueryIndex}) -> maps:fold(fun (PeerId, _, Acc) when PeerId == Id -> Acc; + (_K, #{voter_status := {nonvoter, _}}, Acc) -> + Acc; (_K, #{query_index := Idx}, Acc) -> [Idx | Acc] end, [QueryIndex], Cluster). @@ -2597,6 +2666,8 @@ match_indexes(#{cfg := #cfg{id = Id}, {LWIdx, _} = ra_log:last_written(Log), maps:fold(fun (PeerId, _, Acc) when PeerId == Id -> Acc; + (_K, #{voter_status := {nonvoter, _}}, Acc) -> + Acc; (_K, #{match_index := Idx}, Acc) -> [Idx | Acc] end, [LWIdx], Cluster). @@ -2818,6 +2889,99 @@ meta_name(#cfg{system_config = #{names := #{log_meta := Name}}}) -> Name; meta_name(#{names := #{log_meta := Name}}) -> Name. + +already_member(State) -> + % already a member do nothing + % TODO: reply? If we don't reply the caller may block until timeout + {not_appended, already_member, State}. + +%%% ==================== +%%% Voter status helpers +%%% ==================== + +-spec ensure_promotion_target(ra_voter_status(), ra_server_state()) -> + {ok, ra_voter_status()} | {error, term()}. +ensure_promotion_target({voter, Reason}, _) -> + {ok, {voter, Reason}}; +ensure_promotion_target({nonvoter, #{target := _, nvid := _} = Reason}, _) -> + {ok, {nonvoter, Reason}}; +ensure_promotion_target({nonvoter, #{nvid := _} = Reason}, #{commit_index := CI}) -> + {ok, {nonvoter, Reason#{target => CI}}}; +ensure_promotion_target(_, _) -> + {error, missing_nvid}. + +-spec init_voter_status(ra_server_config() | ra_new_server()) -> ra_voter_status(). +init_voter_status(#{init_non_voter := NVId}) -> + {nonvoter, #{nvid => NVId}}; +init_voter_status(_) -> + {voter, #{}}. + +-spec voter_status(ra_server_state()) -> ra_voter_status(). +voter_status(#{cluster := Cluster} = State) -> + case maps:get(voter_status, State, undefined) of + undefined -> + voter_status(id(State), Cluster); + Voter -> + Voter + end. + +-spec voter_status(ra_server_id(), ra_cluster()) -> ra_voter_status(). +voter_status(PeerId, Cluster) -> + case maps:get(PeerId, Cluster, undefined) of + undefined -> + {undefined, #{}}; + Peer -> + maps:get(voter_status, Peer, {voter, #{}}) + end. + +-spec maybe_promote_self(ra_cluster(), ra_server_state()) -> ra_voter_status(). +maybe_promote_self(NewCluster, State) -> + %% A bit complicated procedure since nvid is not guaranteed. + {_, NReason} = New = voter_status(id(State), NewCluster), + {_, CReason} = Current = voter_status(State), + Self = maps:get(nvid, CReason, undefined), + case maps:get(nvid, NReason, undefined) of + Self -> New; + _ -> Current + end. + +-spec maybe_promote_peer(ra_server_id(), ra_server_state(), effects()) -> effects(). +maybe_promote_peer(PeerID, #{cluster := Cluster}, Effects) -> + % Unknown peer handled in the caller. + #{PeerID := #{match_index := MI, + voter_status := OldStatus}} = Cluster, + case update_voter_status(OldStatus, MI) of + OldStatus -> + Effects; + NewStatus -> + [{next_event, + {command, {'$ra_join', + #{ts => os:system_time(millisecond)}, + #{id => PeerID, voter_status => NewStatus}, + noreply}}} | + Effects] + end. + +update_voter_status({nonvoter, #{target := Target} = Reason}, MI) + when MI >= Target -> + {voter, Reason}; +update_voter_status(Permanent, _) -> + Permanent. + +-spec required_quorum(ra_cluster()) -> pos_integer(). +required_quorum(Cluster) -> + Voters = count_voters(Cluster), + trunc(Voters / 2) + 1. + +count_voters(Cluster) -> + maps:fold( + fun (_, #{voter_status := {nonvoter, _}}, Count) -> + Count; + (_, _, Count) -> + Count + 1 + end, + 0, Cluster). + %%% =================== %%% Internal unit tests %%% =================== diff --git a/src/ra_server_proc.erl b/src/ra_server_proc.erl index 49e8e4d9..7862bfad 100644 --- a/src/ra_server_proc.erl +++ b/src/ra_server_proc.erl @@ -271,7 +271,7 @@ init(Config) -> do_init(#{id := Id, cluster_name := ClusterName} = Config0) -> Key = ra_lib:ra_server_id_to_local_name(Id), - true = ets:insert(ra_state, {Key, init}), + true = ets:insert(ra_state, {Key, init, unknown}), process_flag(trap_exit, true), Config = #{counter := Counter, system_config := SysConf} = maps:merge(config_defaults(Id), @@ -784,11 +784,19 @@ follower(_, tick_timeout, State0) -> set_tick_timer(State, Actions)}; follower({call, From}, {log_fold, Fun, Term}, State) -> fold_log(From, Fun, Term, State); -follower(EventType, Msg, State0) -> +follower(EventType, Msg, #state{conf = #conf{name = Name}, + server_state = SS0} = State0) -> + VoterStatus0 = ra_server:voter_status(SS0), case handle_follower(Msg, State0) of {follower, State1, Effects} -> {State2, Actions} = ?HANDLE_EFFECTS(Effects, EventType, State1), - State = follower_leader_change(State0, State2), + State = #state{server_state = SS} = follower_leader_change(State0, State2), + case ra_server:voter_status(SS) of + VoterStatus0 -> + ok; + VoterStatus -> + true = ets:update_element(ra_state, Name, {3, VoterStatus}) + end, {keep_state, State, Actions}; {pre_vote, State1, Effects} -> {State, Actions} = ?HANDLE_EFFECTS(Effects, EventType, State1), @@ -1029,7 +1037,8 @@ format_status(Opt, [_PDict, StateName, handle_enter(RaftState, OldRaftState, #state{conf = #conf{name = Name}, server_state = ServerState0} = State) -> - true = ets:insert(ra_state, {Name, RaftState}), + VoterStatus = ra_server:voter_status(ServerState0), + true = ets:insert(ra_state, {Name, RaftState, VoterStatus}), {ServerState, Effects} = ra_server:handle_state_enter(RaftState, ServerState0), case RaftState == leader orelse OldRaftState == leader of @@ -1717,9 +1726,11 @@ handle_tick_metrics(State) -> _ = ets:insert(ra_metrics, Metrics), State. -can_execute_locally(RaftState, TargetNode, State) -> +can_execute_locally(RaftState, TargetNode, + #state{server_state = ServerState} = State) -> + {Voter, _} = ra_server:voter_status(ServerState), case RaftState of - follower -> + follower when Voter == voter -> TargetNode == node(); leader when TargetNode =/= node() -> %% We need to evaluate whether to send the message. diff --git a/test/coordination_SUITE.erl b/test/coordination_SUITE.erl index a2c1c630..d163295c 100644 --- a/test/coordination_SUITE.erl +++ b/test/coordination_SUITE.erl @@ -29,6 +29,9 @@ all() -> all_tests() -> [ + nonvoter_catches_up, + nonvoter_catches_up_after_restart, + nonvoter_catches_up_after_leader_restart, start_stop_restart_delete_on_remote, start_cluster, start_or_restart_cluster, @@ -393,6 +396,118 @@ disconnected_node_catches_up(Config) -> [ok = slave:stop(S) || {_, S} <- ServerIds], ok. +nonvoter_catches_up(Config) -> + PrivDir = ?config(data_dir, Config), + ClusterName = ?config(cluster_name, Config), + [A, B, C = {Group, NodeC}] = ServerIds = [{ClusterName, start_follower(N, PrivDir)} || N <- [s1,s2,s3]], + Machine = {module, ?MODULE, #{}}, + {ok, Started, []} = ra:start_cluster(?SYS, ClusterName, Machine, [A, B]), + {ok, _, Leader} = ra:members(hd(Started)), + + [ok = ra:pipeline_command(Leader, N, no_correlation, normal) + || N <- lists:seq(1, 10000)], + {ok, _, _} = ra:process_command(Leader, banana), + + New = #{id => C, init_non_voter => <<"test">>}, + {ok, _, _} = ra:add_member(A, New), + ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), + NonVoter = {nonvoter, #{nvid => <<"test">>}}, + ?assertMatch(#{Group := #{voter_status := NonVoter}}, + rpc:call(NodeC, ra_directory, overview, [?SYS])), + ?assertMatch(#{voter_status := NonVoter}, + ra:key_metrics(C)), + ?assertMatch({ok, #{voter_status := NonVoter}, _}, + ra:member_overview(C)), + + await_condition( + fun () -> + {ok, #{voter_status := {voter, #{nvid := <<"test">>}}}, _} = ra:member_overview(C), + true + end, 200), + ?assertMatch(#{Group := #{voter_status := {voter, #{nvid := <<"test">>}}}}, + rpc:call(NodeC, ra_directory, overview, [?SYS])), + ?assertMatch(#{voter_status := {voter, #{nvid := <<"test">>}}}, + ra:key_metrics(C)), + + [ok = slave:stop(S) || {_, S} <- ServerIds], + ok. + +nonvoter_catches_up_after_restart(Config) -> + PrivDir = ?config(data_dir, Config), + ClusterName = ?config(cluster_name, Config), + [A, B, C = {Group, NodeC}] = ServerIds = [{ClusterName, start_follower(N, PrivDir)} || N <- [s1,s2,s3]], + Machine = {module, ?MODULE, #{}}, + {ok, Started, []} = ra:start_cluster(?SYS, ClusterName, Machine, [A, B]), + {ok, _, Leader} = ra:members(hd(Started)), + + [ok = ra:pipeline_command(Leader, N, no_correlation, normal) + || N <- lists:seq(1, 10000)], + {ok, _, _} = ra:process_command(Leader, banana), + + New = #{id => C, init_non_voter => <<"test">>}, + {ok, _, _} = ra:add_member(A, New), + ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), + NonVoter = {nonvoter, #{nvid => <<"test">>}}, + ?assertMatch(#{Group := #{voter_status := NonVoter}}, + rpc:call(NodeC, ra_directory, overview, [?SYS])), + ?assertMatch(#{voter_status := NonVoter}, + ra:key_metrics(C)), + ?assertMatch({ok, #{voter_status := NonVoter}, _}, + ra:member_overview(C)), + ok = ra:stop_server(?SYS, C), + ok = ra:restart_server(?SYS, C), + + await_condition( + fun () -> + {ok, #{voter_status := {voter, #{nvid := <<"test">>}}}, _} = ra:member_overview(C), + true + end, 200), + ?assertMatch(#{Group := #{voter_status := {voter, #{nvid := <<"test">>}}}}, + rpc:call(NodeC, ra_directory, overview, [?SYS])), + ?assertMatch(#{voter_status := {voter, #{nvid := <<"test">>}}}, + ra:key_metrics(C)), + + [ok = slave:stop(S) || {_, S} <- ServerIds], + ok. + +nonvoter_catches_up_after_leader_restart(Config) -> + PrivDir = ?config(data_dir, Config), + ClusterName = ?config(cluster_name, Config), + [A, B, C = {Group, NodeC}] = ServerIds = [{ClusterName, start_follower(N, PrivDir)} || N <- [s1,s2,s3]], + Machine = {module, ?MODULE, #{}}, + {ok, Started, []} = ra:start_cluster(?SYS, ClusterName, Machine, [A, B]), + {ok, _, Leader} = ra:members(hd(Started)), + + [ok = ra:pipeline_command(Leader, N, no_correlation, normal) + || N <- lists:seq(1, 10000)], + {ok, _, _} = ra:process_command(Leader, banana), + + New = #{id => C, init_non_voter => <<"test">>}, + {ok, _, _} = ra:add_member(A, New), + ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), + NonVoter = {nonvoter, #{nvid => <<"test">>}}, + ?assertMatch(#{Group := #{voter_status := NonVoter}}, + rpc:call(NodeC, ra_directory, overview, [?SYS])), + ?assertMatch(#{voter_status := NonVoter}, + ra:key_metrics(C)), + ?assertMatch({ok, #{voter_status := NonVoter}, _}, + ra:member_overview(C)), + ok = ra:stop_server(?SYS, Leader), + ok = ra:restart_server(?SYS, Leader), + + await_condition( + fun () -> + {ok, #{voter_status := {voter, #{nvid := <<"test">>}}}, _} = ra:member_overview(C), + true + end, 200), + ?assertMatch(#{Group := #{voter_status := {voter, #{nvid := <<"test">>}}}}, + rpc:call(NodeC, ra_directory, overview, [?SYS])), + ?assertMatch(#{voter_status := {voter, #{nvid := <<"test">>}}}, + ra:key_metrics(C)), + + [ok = slave:stop(S) || {_, S} <- ServerIds], + ok. + key_metrics(Config) -> PrivDir = ?config(data_dir, Config), ClusterName = ?config(cluster_name, Config), diff --git a/test/ra_2_SUITE.erl b/test/ra_2_SUITE.erl index 07a7eda8..afe3c01e 100644 --- a/test/ra_2_SUITE.erl +++ b/test/ra_2_SUITE.erl @@ -45,6 +45,7 @@ all_tests() -> external_reader, add_member_without_quorum, force_start_follower_as_single_member, + force_start_follower_as_single_member_nonvoter, initial_members_query ]. @@ -673,7 +674,6 @@ force_start_follower_as_single_member(Config) -> UId4 = ?config(uid4, Config), Conf4 = conf(ClusterName, UId4, ServerId4, PrivDir, [ServerId3]), {ok, _, _} = ra:add_member(ServerId3, ServerId4), - %% the membership has changed but member not running yet {timeout,_} = ra:process_command(ServerId3, {enq, banana}), %% start new member ok = ra:start_server(?SYS, Conf4), @@ -682,6 +682,48 @@ force_start_follower_as_single_member(Config) -> ok. +force_start_follower_as_single_member_nonvoter(Config) -> + ok = logger:set_primary_config(level, all), + %% ra:start_server should fail if the node already exists + ClusterName = ?config(cluster_name, Config), + PrivDir = ?config(priv_dir, Config), + ServerId1 = ?config(server_id, Config), + ServerId2 = ?config(server_id2, Config), + ServerId3 = ?config(server_id3, Config), + InitialCluster = [ServerId1, ServerId2, ServerId3], + ok = start_cluster(ClusterName, InitialCluster), + timer:sleep(100), + %% stop majority to simulate permanent outage + ok = ra:stop_server(?SYS, ServerId1), + ok = ra:stop_server(?SYS, ServerId2), + + timer:sleep(100), + %% force the remaining node to change it's membership + ok = ra_server_proc:force_shrink_members_to_current_member(ServerId3), + {ok, [_], ServerId3} = ra:members(ServerId3), + ok = enqueue(ServerId3, msg1), + + %% test that it works after restart + ok = ra:stop_server(?SYS, ServerId3), + ok = ra:restart_server(?SYS, ServerId3), + {ok, [_], ServerId3} = ra:members(ServerId3), + ok = enqueue(ServerId3, msg2), + + %% add a member + ServerId4 = ?config(server_id4, Config), + UId4 = ?config(uid4, Config), + Conf4 = conf(ClusterName, UId4, ServerId4, PrivDir, [ServerId3]), + {ok, _, _} = ra:add_member(ServerId3, #{id => ServerId4, init_non_voter => <<"test">>}), + %% the membership has changed but member not running yet + %% it is nonvoter and does not affect quorum size + {ok, _, _} = ra:process_command(ServerId3, {enq, banana}), + %% start new member + ok = ra:start_server(?SYS, Conf4#{init_non_voter => <<"test">>}), + {ok, _, ServerId3} = ra:members(ServerId4), + ok = enqueue(ServerId3, msg3), + + ok. + initial_members_query(Config) -> ok = logger:set_primary_config(level, all), %% ra:start_server should fail if the node already exists diff --git a/test/ra_SUITE.erl b/test/ra_SUITE.erl index 0e29fc35..8a6f52de 100644 --- a/test/ra_SUITE.erl +++ b/test/ra_SUITE.erl @@ -64,7 +64,10 @@ all_tests() -> post_partition_liveness, all_metrics_are_integers, transfer_leadership, - transfer_leadership_two_node + transfer_leadership_two_node, + new_nonvoter_knows_its_status, + voter_gets_promoted_consistent_leader, + voter_gets_promoted_new_leader ]. groups() -> @@ -1024,6 +1027,81 @@ transfer_leadership_two_node(Config) -> ?assertEqual({error, unknown_member}, ra:transfer_leadership(NewLeader, {unknown, node()})), terminate_cluster(Members). +new_nonvoter_knows_its_status(Config) -> + Name = ?config(test_name, Config), + [N1, N2] = [{n1, node()}, {n2, node()}], + {ok, _, _} = ra:start_cluster(default, Name, add_machine(), [N1]), + _ = issue_op(N1, 1), + validate_state_on_node(N1, 1), + + % grow + ok = start_and_join_nonvoter(N1, N2), + + % n2 had no time to catch up + % in server state + {ok, #{cluster := #{N1 := #{voter_status := {voter, _}}, + N2 := #{voter_status := {nonvoter, #{target := 2, nvid := T}}}}}, _} = ra:member_overview(N1), + {ok, #{cluster := #{N1 := #{voter_status := {voter, _}}, + N2 := #{voter_status := {nonvoter, #{nvid := T}}}}}, _} = ra:member_overview(N2), + % in ets + #{servers := #{n1 := #{voter_status := {voter, _}}, + n2 := #{voter_status := {nonvoter, _}}}} = ra:overview(?SYS), + ok. + +voter_gets_promoted_consistent_leader(Config) -> + N1 = nth_server_name(Config, 1), + N2 = nth_server_name(Config, 2), + N3 = nth_server_name(Config, 3), + + {ok, _, _} = ra:start_cluster(default, ?config(test_name, Config), add_machine(), [N1]), + _ = issue_op(N1, 1), + validate_state_on_node(N1, 1), + + % grow 1 + ok = start_and_join_nonvoter(N1, N2), + _ = issue_op(N2, 1), + validate_state_on_node(N2, 2), + + % grow 2 + ok = start_and_join_nonvoter(N1, N3), + _ = issue_op(N3, 1), + validate_state_on_node(N3, 3), + + % all are voters after catch-up + timer:sleep(100), + All = [N1, N2, N3], + % in server state + lists:map(fun(O) -> ?assertEqual(All, voters(O)) end, overviews(N1)), + % in ets + #{servers := Servers} = ra:overview(?SYS), + lists:map(fun({Name, _}) -> #{Name := #{voter_status := {voter, _}}} = Servers end, All), + ok. + +voter_gets_promoted_new_leader(Config) -> + N1 = nth_server_name(Config, 1), + N2 = nth_server_name(Config, 2), + N3 = nth_server_name(Config, 3), + + {ok, [Leader, _Second], []} = ra:start_cluster(default, ?config(test_name, Config), add_machine(), [N1, N2]), + _ = issue_op(N1, 1), + validate_state_on_node(N1, 1), + + % grow with leadership change + ok = start_and_join_nonvoter(N1, N3), + ra:transfer_leadership(Leader, _Second), + _ = issue_op(N3, 1), + validate_state_on_node(N3, 2), + + % all are voters after catch-up + timer:sleep(100), + All = [N1, N2, N3], + % in server state + lists:map(fun(O) -> ?assertEqual(All, voters(O)) end, overviews(N1)), + % in ets + #{servers := Servers} = ra:overview(?SYS), + lists:map(fun({Name, _}) -> #{Name := #{voter_status := {voter, _}}} = Servers end, All), + ok. + get_gen_statem_status(Ref) -> {_, _, _, Items} = sys:get_status(Ref), proplists:get_value(raft_state, lists:last(Items)). @@ -1096,6 +1174,12 @@ start_and_join({ClusterName, _} = ServerRef, {_, _} = New) -> ok = ra:start_server(default, ClusterName, New, add_machine(), [ServerRef]), ok. +start_and_join_nonvoter({ClusterName, _} = ServerRef, {_, _} = New) -> + Server = #{id => New, init_non_voter => <<"test">>}, + {ok, _, _} = ra:add_member(ServerRef, Server), + ok = ra:start_server(default, ClusterName, Server, add_machine(), [ServerRef]), + ok. + start_local_cluster(Num, Name, Machine) -> Nodes = [{ra_server:name(Name, integer_to_list(N)), node()} || N <- lists:seq(1, Num)], @@ -1126,6 +1210,16 @@ nth_server_name(Config, N) when is_integer(N) -> add_machine() -> {module, ?MODULE, #{}}. +overviews(Node) -> + {ok, Members, _From} = ra:members(Node), + [ra:member_overview(P) || {_, _} = P <- Members]. + +voters({ok, #{cluster := Peers}, _} = _Overview) -> + [Id || {Id, Status} <- maps:to_list(Peers), begin + {Voter, _} = maps:get(voter_status, Status), + Voter == voter + end]. + %% machine impl init(_) -> 0. apply(_Meta, Num, State) -> diff --git a/test/ra_log_snapshot_SUITE.erl b/test/ra_log_snapshot_SUITE.erl index 0fa3fd66..cd401a78 100644 --- a/test/ra_log_snapshot_SUITE.erl +++ b/test/ra_log_snapshot_SUITE.erl @@ -198,4 +198,5 @@ meta(Idx, Term, Cluster) -> #{index => Idx, term => Term, cluster => Cluster, + non_voters => maps:from_list([{N, #{}} || N <- Cluster]), machine_version => 1}. diff --git a/test/ra_server_SUITE.erl b/test/ra_server_SUITE.erl index 7b252bea..1dbf65c2 100644 --- a/test/ra_server_SUITE.erl +++ b/test/ra_server_SUITE.erl @@ -20,6 +20,7 @@ all() -> follower_aer_term_mismatch_snapshot, follower_handles_append_entries_rpc, candidate_handles_append_entries_rpc, + append_entries_reply_success_promotes_nonvoter, append_entries_reply_success, append_entries_reply_no_success, follower_request_vote, @@ -41,10 +42,12 @@ all() -> follower_machine_version, follower_install_snapshot_machine_version, leader_server_join, + leader_server_join_nonvoter, leader_server_leave, leader_is_removed, follower_cluster_change, leader_applies_new_cluster, + leader_applies_new_cluster_nonvoter, leader_appends_cluster_change_then_steps_before_applying_it, leader_receives_install_snapshot_rpc, follower_installs_snapshot, @@ -54,6 +57,7 @@ all() -> snapshotted_follower_received_append_entries, leader_received_append_entries_reply_with_stale_last_index, leader_receives_install_snapshot_result, + leader_received_append_entries_reply_and_promotes_voter, leader_replies_to_append_entries_rpc_with_lower_term, follower_aer_1, follower_aer_2, @@ -275,6 +279,10 @@ election_timeout(_Config) -> {N3, _}]}]} = ra_server:handle_follower(Msg, State), + % non-voters ignore election_timeout + NVState = State#{voter_status => {nonvoter, test}}, + {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), + % pre_vote {pre_vote, #{current_term := 5, votes := 0, pre_vote_token := Token1}, @@ -796,6 +804,89 @@ candidate_handles_append_entries_rpc(_Config) -> = ra_server:handle_candidate(EmptyAE, State), ok. +append_entries_reply_success_promotes_nonvoter(_Config) -> + N1 = ?N1, N2 = ?N2, N3 = ?N3, + NonVoter = {nonvoter, #{target => 3, nvid => <<"test">>}}, + Cluster = #{N1 => new_peer_with(#{next_index => 5, match_index => 4}), + N2 => new_peer_with(#{next_index => 1, match_index => 0, + commit_index_sent => 3, + voter_status => NonVoter}), + N3 => new_peer_with(#{next_index => 2, match_index => 1})}, + State0 = (base_state(3, ?FUNCTION_NAME))#{commit_index => 1, + last_applied => 1, + cluster => Cluster, + machine_state => <<"hi1">>}, + Ack = #append_entries_reply{term = 5, success = true, + next_index = 4, + last_index = 3, last_term = 5}, + + % doesn't progress commit_index, non voter ack doesn't raise majority + {leader, #{cluster := #{N2 := #{next_index := 4, + match_index := 3, + voter_status := NonVoter}}, + commit_index := 1, + last_applied := 1, + machine_state := <<"hi1">>} = State1, + [{next_event, info, pipeline_rpcs}, + {next_event, {command, {'$ra_join', _, + #{id := N2, voter_status := {voter, _}}, noreply}} = RaJoin} + ]} = ra_server:handle_leader({N2, Ack}, State0), + + % pipeline to N3 + {leader, #{cluster := #{N3 := #{next_index := 4, + match_index := 1}}, + commit_index := 1, + last_applied := 1, + machine_state := <<"hi1">>} = State2, + [{send_rpc, N3, + #append_entries_rpc{term = 5, leader_id = N1, + prev_log_index = 1, + prev_log_term = 1, + leader_commit = 1, + entries = [{2, 3, {'$usr', _, <<"hi2">>, _}}, + {3, 5, {'$usr', _, <<"hi3">>, _}}]} + }]} = ra_server:handle_leader(pipeline_rpcs, State1), + + % ra_join translates into cluster update + {leader, #{cluster := #{N2 := #{next_index := 5, + match_index := 3, + voter_status := {voter, _}}}, + cluster_change_permitted := false, + commit_index := 1, + last_applied := 1, + machine_state := <<"hi1">>} = State3, + [{send_rpc, N3, + #append_entries_rpc{term = 5, leader_id = N1, + prev_log_index = 3, + prev_log_term = 5, + leader_commit = 1, + entries = [{4, 5, {'$ra_cluster_change', _, + #{N2 := #{voter_status := {voter, _}}}, + _}}]}}, + {send_rpc, N2, + #append_entries_rpc{term = 5, leader_id = N1, + prev_log_index = 3, + prev_log_term = 5, + leader_commit = 1, + entries = [{4, 5, {'$ra_cluster_change', _, + #{N2 := #{voter_status := {voter, _}}}, + _}}]}} + ]} = ra_server:handle_leader(RaJoin, State2), + + Ack2 = #append_entries_reply{term = 5, success = true, + next_index = 5, + last_index = 4, last_term = 5}, + + % voter ack, raises commit_index + {leader, #{cluster := #{N2 := #{next_index := 5, + match_index := 4}}, + commit_index := 3, + last_applied := 3, + machine_state := <<"hi3">>}, + [{next_event, info, pipeline_rpcs}, + {aux, eval}]} = ra_server:handle_leader({N2, Ack2}, State3), + ok. + append_entries_reply_success(_Config) -> N1 = ?N1, N2 = ?N2, N3 = ?N3, @@ -918,6 +1009,11 @@ follower_request_vote(_Config) -> [{reply, #request_vote_result{term = 6, vote_granted = true}}]} = ra_server:handle_follower(Msg#request_vote_rpc{last_log_index = 4}, State), + + % non-voters ignore request_vote_rpc + NVState = State#{voter_status => {nonvoter, test}}, + {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), + ok. follower_pre_vote(_Config) -> @@ -1032,6 +1128,11 @@ follower_pre_vote(_Config) -> vote_granted = true}}]} = ra_server:handle_follower(Msg#pre_vote_rpc{last_log_index = 4}, State), + + % non-voters ignore pre_vote_rpc + NVState = State#{voter_status => {nonvoter, test}}, + {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), + ok. pre_vote_receives_pre_vote(_Config) -> @@ -1305,17 +1406,60 @@ leader_server_join(_Config) -> cluster_change_permitted := false} = _State1, Effects} = ra_server:handle_leader({command, {'$ra_join', meta(), N4, await_consensus}}, State0), + % new member should join as voter + [ + {send_rpc, N4, + #append_entries_rpc{entries = + [_, _, _, {4, 5, {'$ra_cluster_change', _, + #{N1 := _, N2 := _, + N3 := _, N4 := #{voter_status := {voter, _}}}, + await_consensus}}]}}, + {send_rpc, N3, + #append_entries_rpc{entries = + [{4, 5, {'$ra_cluster_change', _, + #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := {voter, _}}}, + await_consensus}}], + term = 5, leader_id = N1, + prev_log_index = 3, + prev_log_term = 5, + leader_commit = 3}}, + {send_rpc, N2, + #append_entries_rpc{entries = + [{4, 5, {'$ra_cluster_change', _, + #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := {voter, _}}}, + await_consensus}}], + term = 5, leader_id = N1, + prev_log_index = 3, + prev_log_term = 5, + leader_commit = 3}} + | _] = Effects, + ok. + +leader_server_join_nonvoter(_Config) -> + N1 = ?N1, N2 = ?N2, N3 = ?N3, N4 = ?N4, + OldCluster = #{N1 => new_peer_with(#{next_index => 4, match_index => 3}), + N2 => new_peer_with(#{next_index => 4, match_index => 3}), + N3 => new_peer_with(#{next_index => 4, match_index => 3})}, + State0 = (base_state(3, ?FUNCTION_NAME))#{cluster => OldCluster}, + % raft servers should switch to the new configuration after log append + % and further cluster changes should be disallowed + {leader, #{cluster := #{N1 := _, N2 := _, N3 := _, N4 := _}, + commit_index := Target, + cluster_change_permitted := false} = _State1, Effects} = + ra_server:handle_leader({command, {'$ra_join', meta(), + #{id => N4, init_non_voter => <<"test">>}, await_consensus}}, State0), + % new member should join as non-voter [ {send_rpc, N4, #append_entries_rpc{entries = [_, _, _, {4, 5, {'$ra_cluster_change', _, #{N1 := _, N2 := _, - N3 := _, N4 := _}, + N3 := _, N4 := #{voter_status := {nonvoter, #{target := Target}}}}, await_consensus}}]}}, {send_rpc, N3, #append_entries_rpc{entries = [{4, 5, {'$ra_cluster_change', _, - #{N1 := _, N2 := _, N3 := _, N4 := _}, + #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := {nonvoter, #{target := Target}}}}, await_consensus}}], term = 5, leader_id = N1, prev_log_index = 3, @@ -1324,7 +1468,7 @@ leader_server_join(_Config) -> {send_rpc, N2, #append_entries_rpc{entries = [{4, 5, {'$ra_cluster_change', _, - #{N1 := _, N2 := _, N3 := _, N4 := _}, + #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := {nonvoter, #{target := Target}}}}, await_consensus}}], term = 5, leader_id = N1, prev_log_index = 3, @@ -1438,13 +1582,11 @@ leader_applies_new_cluster(_Config) -> ?assert(not maps:get(cluster_change_permitted, State2)), - % replies coming in AEReply = #append_entries_reply{term = 5, success = true, next_index = 5, last_index = 4, last_term = 5}, % leader does not yet have consensus as will need at least 3 votes {leader, State3 = #{commit_index := 3, - cluster_change_permitted := false, cluster_index_term := {4, 5}, cluster := #{N2 := #{next_index := 5, @@ -1457,6 +1599,37 @@ leader_applies_new_cluster(_Config) -> cluster := #{N3 := #{next_index := 5, match_index := 4}}}, _Effects} = ra_server:handle_leader({N3, AEReply}, State3), + ok. + +leader_applies_new_cluster_nonvoter(_Config) -> + N1 = ?N1, N2 = ?N2, N3 = ?N3, N4 = ?N4, + OldCluster = #{N1 => new_peer_with(#{next_index => 4, match_index => 3}), + N2 => new_peer_with(#{next_index => 4, match_index => 3}), + N3 => new_peer_with(#{next_index => 4, match_index => 3})}, + + State = (base_state(3, ?FUNCTION_NAME))#{cluster => OldCluster}, + Command = {command, {'$ra_join', meta(), #{id => N4, init_non_voter => <<"test">>}, await_consensus}}, + % cluster records index and term it was applied to determine whether it has + % been applied + {leader, #{cluster_index_term := {4, 5}, + cluster := #{N1 := _, N2 := _, + N3 := _, N4 := _} } = State1, _} = + ra_server:handle_leader(Command, State), + {leader, State2, _} = + ra_server:handle_leader(written_evt({4, 4, 5}), State1), + + ?assert(not maps:get(cluster_change_permitted, State2)), + + % replies coming in + AEReply = #append_entries_reply{term = 5, success = true, + next_index = 5, + last_index = 4, last_term = 5}, + % new peer doesn't count until it reaches its matching target, leader needs only 2 votes + {leader, _State3 = #{commit_index := 4, + cluster_change_permitted := true, + cluster := #{N2 := #{next_index := 5, + match_index := 4}}}, + _} = ra_server:handle_leader({N2, AEReply}, State2#{votes => 1}), ok. leader_appends_cluster_change_then_steps_before_applying_it(_Config) -> @@ -1703,8 +1876,7 @@ follower_installs_snapshot(_Config) -> Term = 2, % leader term Idx = 3, ISRpc = #install_snapshot_rpc{term = Term, leader_id = N1, - meta = snap_meta(Idx, LastTerm, - maps:keys(Config)), + meta = snap_meta(Idx, LastTerm, Config), chunk_state = {1, last}, data = []}, {receive_snapshot, FState1, @@ -1716,10 +1888,10 @@ follower_installs_snapshot(_Config) -> {#{index => Idx, term => Term, cluster => maps:keys(Config), + cluster_state => Config, machine_version => 0}, []} end), - {follower, #{current_term := Term, commit_index := Idx, last_applied := Idx, @@ -1761,8 +1933,7 @@ follower_receives_stale_snapshot(_Config) -> LastTerm = 1, % snapshot term Idx = 2, ISRpc = #install_snapshot_rpc{term = CurTerm, leader_id = N1, - meta = snap_meta(Idx, LastTerm, - maps:keys(Config)), + meta = snap_meta(Idx, LastTerm, Config), chunk_state = {1, last}, data = []}, %% this should be a rare occurrence, rather than implement a special @@ -1780,8 +1951,7 @@ receive_snapshot_timeout(_Config) -> LastTerm = 1, % snapshot term Idx = 6, ISRpc = #install_snapshot_rpc{term = CurTerm, leader_id = N1, - meta = snap_meta(Idx, LastTerm, - maps:keys(Config)), + meta = snap_meta(Idx, LastTerm, Config), chunk_state = {1, last}, data = []}, {receive_snapshot, FState1, @@ -1812,8 +1982,7 @@ snapshotted_follower_received_append_entries(_Config) -> []} end), ISRpc = #install_snapshot_rpc{term = Term, leader_id = N1, - meta = snap_meta(Idx, LastTerm, - maps:keys(Config)), + meta = snap_meta(Idx, LastTerm, Config), chunk_state = {1, last}, data = []}, {follower, FState1, _} = ra_server:handle_receive_snapshot(ISRpc, FState0), @@ -1949,6 +2118,32 @@ leader_receives_install_snapshot_result(_Config) -> (_) -> false end, Effects)), ok. +leader_received_append_entries_reply_and_promotes_voter(_config) -> + N3 = ?N3, State = base_state(3, ?FUNCTION_NAME), + AER = #append_entries_reply{term = 5, success = true, + next_index = 5, + last_index = 4, last_term = 5}, + + % Permanent voter + State1 = set_peer_voter_status(State, N3, {voter, #{nvid => <<"test">>}}), + {leader, _, + [{next_event,info,pipeline_rpcs}] + } = ra_server:handle_leader({N3, AER}, State1), + + % Permanent non-voter + State2 = set_peer_voter_status(State, N3, {nonvoter, test}), + {leader, _, + [{next_event,info,pipeline_rpcs}] + } = ra_server:handle_leader({N3, AER}, State2), + + % Promotion + State3 = set_peer_voter_status(State, N3, + {nonvoter, #{target => 4}}), + {leader, _, + [{next_event,info,pipeline_rpcs}, + {next_event, {command, {'$ra_join', _, #{id := N3, voter_status := {voter, _}}, _}}}] + } = ra_server:handle_leader({N3, AER}, State3). + follower_heartbeat(_Config) -> State = base_state(3, ?FUNCTION_NAME), #{current_term := Term, @@ -2456,6 +2651,11 @@ set_peer_query_index(State, PeerId, QueryIndex) -> #{PeerId := Peer} = Cluster, State#{cluster := Cluster#{PeerId => Peer#{query_index => QueryIndex}}}. +set_peer_voter_status(State, PeerId, VoterStatus) -> + #{cluster := Cluster} = State, + #{PeerId := Peer} = Cluster, + State#{cluster := Cluster#{PeerId => Peer#{voter_status => VoterStatus}}}. + leader_heartbeat_reply_lower_term(_Config) -> State = base_state(3, ?FUNCTION_NAME), #{current_term := Term, @@ -2593,17 +2793,19 @@ new_peer() -> match_index => 0, query_index => 0, commit_index_sent => 0, + voter_status => {voter, #{nvid => <<"test">>}}, status => normal}. new_peer_with(Map) -> maps:merge(new_peer(), Map). snap_meta(Idx, Term) -> - snap_meta(Idx, Term, []). + snap_meta(Idx, Term, #{}). snap_meta(Idx, Term, Cluster) -> #{index => Idx, term => Term, - cluster => Cluster, + cluster => maps:keys(Cluster), + cluster_state => Cluster, machine_version => 0}. From 27a533b33e0ac3b411ecf86b1950ff154ff58036 Mon Sep 17 00:00:00 2001 From: Alex Valiushko Date: Mon, 18 Sep 2023 15:10:23 -0700 Subject: [PATCH 2/8] address feedback --- src/ra.hrl | 2 +- src/ra_server.erl | 32 ++++++++++++--------------- src/ra_server_proc.erl | 12 ++++++++-- test/coordination_SUITE.erl | 44 +++++++++++++++++++++++++++++-------- test/ra_2_SUITE.erl | 4 ++-- test/ra_SUITE.erl | 2 +- test/ra_server_SUITE.erl | 4 ++-- 7 files changed, 65 insertions(+), 35 deletions(-) diff --git a/src/ra.hrl b/src/ra.hrl index e6a560b6..a48ed14c 100644 --- a/src/ra.hrl +++ b/src/ra.hrl @@ -48,7 +48,7 @@ %% If set, server will start as non-voter until later promoted by the %% leader. - init_non_voter => ra_nvid()}. + non_voter_id => ra_nvid()}. -type ra_peer_status() :: normal | {sending_snapshot, pid()} | diff --git a/src/ra_server.erl b/src/ra_server.erl index 760581a5..e40a039b 100644 --- a/src/ra_server.erl +++ b/src/ra_server.erl @@ -197,7 +197,7 @@ max_pipeline_count => non_neg_integer(), ra_event_formatter => {module(), atom(), [term()]}, counter => counters:counters_ref(), - init_non_voter => ra_nvid(), + non_voter_id => ra_nvid(), system_config => ra_system:config()}. -type mutable_config() :: #{cluster_name => ra_cluster_name(), @@ -2899,19 +2899,21 @@ already_member(State) -> %%% Voter status helpers %%% ==================== --spec ensure_promotion_target(ra_voter_status(), ra_server_state()) -> +-spec ensure_promotion_target(ra_voter_status(), ra_index()) -> {ok, ra_voter_status()} | {error, term()}. ensure_promotion_target({voter, Reason}, _) -> {ok, {voter, Reason}}; ensure_promotion_target({nonvoter, #{target := _, nvid := _} = Reason}, _) -> {ok, {nonvoter, Reason}}; -ensure_promotion_target({nonvoter, #{nvid := _} = Reason}, #{commit_index := CI}) -> - {ok, {nonvoter, Reason#{target => CI}}}; +ensure_promotion_target({nonvoter, #{nvid := _} = Reason}, + #{log := Log}) -> + Target = ra_log:next_index(Log), + {ok, {nonvoter, Reason#{target => Target}}}; ensure_promotion_target(_, _) -> {error, missing_nvid}. -spec init_voter_status(ra_server_config() | ra_new_server()) -> ra_voter_status(). -init_voter_status(#{init_non_voter := NVId}) -> +init_voter_status(#{non_voter_id := NVId}) -> {nonvoter, #{nvid => NVId}}; init_voter_status(_) -> {voter, #{}}. @@ -2949,25 +2951,19 @@ maybe_promote_self(NewCluster, State) -> maybe_promote_peer(PeerID, #{cluster := Cluster}, Effects) -> % Unknown peer handled in the caller. #{PeerID := #{match_index := MI, - voter_status := OldStatus}} = Cluster, - case update_voter_status(OldStatus, MI) of - OldStatus -> - Effects; - NewStatus -> + voter_status := Status}} = Cluster, + case Status of + {nonvoter, #{target := Target} = Reason} when MI >= Target -> [{next_event, {command, {'$ra_join', #{ts => os:system_time(millisecond)}, - #{id => PeerID, voter_status => NewStatus}, + #{id => PeerID, voter_status => {voter, Reason}}, noreply}}} | - Effects] + Effects]; + _ -> + Effects end. -update_voter_status({nonvoter, #{target := Target} = Reason}, MI) - when MI >= Target -> - {voter, Reason}; -update_voter_status(Permanent, _) -> - Permanent. - -spec required_quorum(ra_cluster()) -> pos_integer(). required_quorum(Cluster) -> Voters = count_voters(Cluster), diff --git a/src/ra_server_proc.erl b/src/ra_server_proc.erl index 7862bfad..6e4966b5 100644 --- a/src/ra_server_proc.erl +++ b/src/ra_server_proc.erl @@ -183,6 +183,7 @@ log_fold(ServerId, Fun, InitialState, Timeout) -> -spec state_query(server_loc(), all | overview | + voters | members | initial_members | machine, timeout()) -> @@ -193,6 +194,7 @@ state_query(ServerLoc, Spec, Timeout) -> -spec local_state_query(server_loc(), all | overview | + voters | members | initial_members | machine, timeout()) -> @@ -1519,6 +1521,12 @@ do_state_query(overview, State) -> ra_server:overview(State); do_state_query(machine, #{machine_state := MacState}) -> MacState; +do_state_query(voters, #{cluster := Cluster}) -> + Voters = maps:filter(fun(_, Peer) -> + {Voter, _} = maps:get(voter_status, Peer, {voter, legacy}), + Voter == voter + end, Cluster), + maps:keys(Voters); do_state_query(members, #{cluster := Cluster}) -> maps:keys(Cluster); do_state_query(initial_members, #{log := Log}) -> @@ -1735,8 +1743,8 @@ can_execute_locally(RaftState, TargetNode, leader when TargetNode =/= node() -> %% We need to evaluate whether to send the message. %% Only send if there isn't a local node for the target pid. - Members = do_state_query(members, State#state.server_state), - not lists:any(fun ({_, N}) -> N == TargetNode end, Members); + Voters = do_state_query(voters, State#state.server_state), + not lists:any(fun ({_, N}) -> N == TargetNode end, Voters); leader -> true; _ -> diff --git a/test/coordination_SUITE.erl b/test/coordination_SUITE.erl index d163295c..e0ac392c 100644 --- a/test/coordination_SUITE.erl +++ b/test/coordination_SUITE.erl @@ -298,13 +298,20 @@ start_cluster_minority(Config) -> send_local_msg(Config) -> PrivDir = ?config(data_dir, Config), ClusterName = ?config(cluster_name, Config), - NodeIds = [{ClusterName, start_follower(N, PrivDir)} || N <- [s1,s2,s3]], + [A, B, NonVoter] = [{ClusterName, start_follower(N, PrivDir)} || N <- [s1,s2,s3]], + NodeIds = [A, B], Machine = {module, ?MODULE, #{}}, {ok, Started, []} = ra:start_cluster(?SYS, ClusterName, Machine, NodeIds), % assert all were said to be started [] = Started -- NodeIds, - %% spawn a receiver process on one node + % add permanent non-voter {ok, _, Leader} = ra:members(hd(NodeIds)), + {ok, _, _} = ra:process_command(Leader, banana), + New = #{id => NonVoter, + voter_status => {nonvoter, #{nvid => <<"test">>, target => 999}}, + non_voter_id => <<"test">>}, + {ok, _, _} = ra:add_member(A, New), + ok = ra:start_server(?SYS, ClusterName, New, Machine, NodeIds), %% select a non-leader node to spawn on [{_, N} | _] = lists:delete(Leader, NodeIds), test_local_msg(Leader, N, N, send_local_msg, local), @@ -312,36 +319,55 @@ send_local_msg(Config) -> test_local_msg(Leader, N, N, send_local_msg, [local, cast]), test_local_msg(Leader, N, N, send_local_msg, [local, cast, ra_event]), {_, LeaderNode} = Leader, + %% test the same but for a local pid (non-member) test_local_msg(Leader, node(), LeaderNode, send_local_msg, local), test_local_msg(Leader, node(), LeaderNode, send_local_msg, [local, ra_event]), test_local_msg(Leader, node(), LeaderNode, send_local_msg, [local, cast]), test_local_msg(Leader, node(), LeaderNode, send_local_msg, [local, cast, ra_event]), - %% test the same but for a local pid (non-member) + %% same for non-voter + {_, NonVoterNode} = NonVoter, + test_local_msg(Leader, NonVoterNode, LeaderNode, send_local_msg, local), + test_local_msg(Leader, NonVoterNode, LeaderNode, send_local_msg, [local, ra_event]), + test_local_msg(Leader, NonVoterNode, LeaderNode, send_local_msg, [local, cast]), + test_local_msg(Leader, NonVoterNode, LeaderNode, send_local_msg, [local, cast, ra_event]), [ok = slave:stop(S) || {_, S} <- NodeIds], ok. local_log_effect(Config) -> PrivDir = ?config(data_dir, Config), ClusterName = ?config(cluster_name, Config), - NodeIds = [{ClusterName, start_follower(N, PrivDir)} || N <- [s1,s2,s3]], + [A, B, NonVoter] = [{ClusterName, start_follower(N, PrivDir)} || N <- [s1,s2,s3]], + NodeIds = [A, B], Machine = {module, ?MODULE, #{}}, {ok, Started, []} = ra:start_cluster(?SYS, ClusterName, Machine, NodeIds), % assert all were said to be started [] = Started -- NodeIds, - %% spawn a receiver process on one node + % add permanent non-voter {ok, _, Leader} = ra:members(hd(NodeIds)), + {ok, _, _} = ra:process_command(Leader, banana), + New = #{id => NonVoter, + voter_status => {nonvoter, #{nvid => <<"test">>, target => 999}}, + non_voter_id => <<"test">>}, + {ok, _, _} = ra:add_member(A, New), + ok = ra:start_server(?SYS, ClusterName, New, Machine, NodeIds), %% select a non-leader node to spawn on [{_, N} | _] = lists:delete(Leader, NodeIds), test_local_msg(Leader, N, N, do_local_log, local), test_local_msg(Leader, N, N, do_local_log, [local, ra_event]), test_local_msg(Leader, N, N, do_local_log, [local, cast]), test_local_msg(Leader, N, N, do_local_log, [local, cast, ra_event]), + %% test the same but for a local pid (non-member) {_, LeaderNode} = Leader, test_local_msg(Leader, node(), LeaderNode, do_local_log, local), test_local_msg(Leader, node(), LeaderNode, do_local_log, [local, ra_event]), test_local_msg(Leader, node(), LeaderNode, do_local_log, [local, cast]), test_local_msg(Leader, node(), LeaderNode, do_local_log, [local, cast, ra_event]), - %% test the same but for a local pid (non-member) + %% same for non-voter + {_, NonVoterNode} = NonVoter, + test_local_msg(Leader, NonVoterNode, LeaderNode, do_local_log, local), + test_local_msg(Leader, NonVoterNode, LeaderNode, do_local_log, [local, ra_event]), + test_local_msg(Leader, NonVoterNode, LeaderNode, do_local_log, [local, cast]), + test_local_msg(Leader, NonVoterNode, LeaderNode, do_local_log, [local, cast, ra_event]), [ok = slave:stop(S) || {_, S} <- NodeIds], ok. @@ -408,7 +434,7 @@ nonvoter_catches_up(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, init_non_voter => <<"test">>}, + New = #{id => C, non_voter_id => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), NonVoter = {nonvoter, #{nvid => <<"test">>}}, @@ -444,7 +470,7 @@ nonvoter_catches_up_after_restart(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, init_non_voter => <<"test">>}, + New = #{id => C, non_voter_id => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), NonVoter = {nonvoter, #{nvid => <<"test">>}}, @@ -482,7 +508,7 @@ nonvoter_catches_up_after_leader_restart(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, init_non_voter => <<"test">>}, + New = #{id => C, non_voter_id => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), NonVoter = {nonvoter, #{nvid => <<"test">>}}, diff --git a/test/ra_2_SUITE.erl b/test/ra_2_SUITE.erl index afe3c01e..4fb8bdbc 100644 --- a/test/ra_2_SUITE.erl +++ b/test/ra_2_SUITE.erl @@ -713,12 +713,12 @@ force_start_follower_as_single_member_nonvoter(Config) -> ServerId4 = ?config(server_id4, Config), UId4 = ?config(uid4, Config), Conf4 = conf(ClusterName, UId4, ServerId4, PrivDir, [ServerId3]), - {ok, _, _} = ra:add_member(ServerId3, #{id => ServerId4, init_non_voter => <<"test">>}), + {ok, _, _} = ra:add_member(ServerId3, #{id => ServerId4, non_voter_id => <<"test">>}), %% the membership has changed but member not running yet %% it is nonvoter and does not affect quorum size {ok, _, _} = ra:process_command(ServerId3, {enq, banana}), %% start new member - ok = ra:start_server(?SYS, Conf4#{init_non_voter => <<"test">>}), + ok = ra:start_server(?SYS, Conf4#{non_voter_id => <<"test">>}), {ok, _, ServerId3} = ra:members(ServerId4), ok = enqueue(ServerId3, msg3), diff --git a/test/ra_SUITE.erl b/test/ra_SUITE.erl index 8a6f52de..d3481339 100644 --- a/test/ra_SUITE.erl +++ b/test/ra_SUITE.erl @@ -1175,7 +1175,7 @@ start_and_join({ClusterName, _} = ServerRef, {_, _} = New) -> ok. start_and_join_nonvoter({ClusterName, _} = ServerRef, {_, _} = New) -> - Server = #{id => New, init_non_voter => <<"test">>}, + Server = #{id => New, non_voter_id => <<"test">>}, {ok, _, _} = ra:add_member(ServerRef, Server), ok = ra:start_server(default, ClusterName, Server, add_machine(), [ServerRef]), ok. diff --git a/test/ra_server_SUITE.erl b/test/ra_server_SUITE.erl index 1dbf65c2..dcf7604d 100644 --- a/test/ra_server_SUITE.erl +++ b/test/ra_server_SUITE.erl @@ -1447,7 +1447,7 @@ leader_server_join_nonvoter(_Config) -> commit_index := Target, cluster_change_permitted := false} = _State1, Effects} = ra_server:handle_leader({command, {'$ra_join', meta(), - #{id => N4, init_non_voter => <<"test">>}, await_consensus}}, State0), + #{id => N4, non_voter_id => <<"test">>}, await_consensus}}, State0), % new member should join as non-voter [ {send_rpc, N4, @@ -1608,7 +1608,7 @@ leader_applies_new_cluster_nonvoter(_Config) -> N3 => new_peer_with(#{next_index => 4, match_index => 3})}, State = (base_state(3, ?FUNCTION_NAME))#{cluster => OldCluster}, - Command = {command, {'$ra_join', meta(), #{id => N4, init_non_voter => <<"test">>}, await_consensus}}, + Command = {command, {'$ra_join', meta(), #{id => N4, non_voter_id => <<"test">>}, await_consensus}}, % cluster records index and term it was applied to determine whether it has % been applied {leader, #{cluster_index_term := {4, 5}, From d854c5654f4449caf4f7344a33e5dbe7c2ff73ea Mon Sep 17 00:00:00 2001 From: Alex Valiushko Date: Tue, 19 Sep 2023 17:01:43 -0700 Subject: [PATCH 3/8] use uid for identity --- src/ra.erl | 19 +--- src/ra.hrl | 17 ++- src/ra_directory.erl | 2 +- src/ra_server.erl | 188 ++++++++++++++++----------------- src/ra_server_proc.erl | 30 +++--- test/coordination_SUITE.erl | 61 ++++++----- test/ra_2_SUITE.erl | 5 +- test/ra_SUITE.erl | 26 ++--- test/ra_log_snapshot_SUITE.erl | 1 - test/ra_server_SUITE.erl | 65 ++++++------ 10 files changed, 200 insertions(+), 214 deletions(-) diff --git a/src/ra.erl b/src/ra.erl index ff0ba778..a7995aa9 100644 --- a/src/ra.erl +++ b/src/ra.erl @@ -71,7 +71,6 @@ overview/1, %% helpers new_uid/1, - new_nvid/0, %% rebalancing transfer_leadership/2, %% auxiliary commands @@ -484,7 +483,7 @@ start_server(System, ClusterName, #{id := {_, _}} = Conf0, Machine, ServerIds) initial_members => ServerIds, log_init_args => #{uid => UId}, machine => Machine}, - start_server(System, maps:merge(Conf0, Conf)). + start_server(System, maps:merge(Conf, Conf0)). %% @doc Starts a ra server in the default system %% @param Conf a ra_server_config() configuration map. @@ -719,14 +718,6 @@ new_uid(Source) when is_binary(Source) -> Prefix = ra_lib:derive_safe_string(Source, 6), ra_lib:make_uid(string:uppercase(Prefix)). -%% @doc generates a random uid using timestamp for the first -%% 6 characters. -%% @end -new_nvid() -> - Millis = erlang:system_time(millisecond), - Prefix = base64:encode(<>), - new_uid(Prefix). - %% @doc Returns a map of overview data of the default Ra system on the current Erlang %% node. %% DEPRECATED: user overview/1 @@ -1143,15 +1134,15 @@ key_metrics({Name, N} = ServerId) when N == node() -> case whereis(Name) of undefined -> Counters#{state => noproc, - voter_status => noproc}; + non_voter => noproc}; _ -> case ets:lookup(ra_state, Name) of [] -> Counters#{state => unknown, - voter_status => unknown}; - [{_, State, Voter}] -> + non_voter => unknown}; + [{_, State, NonVoter}] -> Counters#{state => State, - voter_status => Voter} + non_voter => NonVoter} end end; key_metrics({_, N} = ServerId) -> diff --git a/src/ra.hrl b/src/ra.hrl index a48ed14c..7ad277d0 100644 --- a/src/ra.hrl +++ b/src/ra.hrl @@ -33,9 +33,6 @@ %% used for on disk resources and local name to pid mapping -type ra_uid() :: binary(). -%% Transient ID that uniquely identifies any new non-voter. --type ra_nvid() :: binary(). - %% Identifies a Ra server (node) in a Ra cluster. %% %% Ra servers need to be registered stable names (names that are reachable @@ -43,23 +40,23 @@ -type ra_server_id() :: {Name :: atom(), Node :: node()}. %% Specifies server configuration for a new cluster member. -%% Both `ra:add_member` and `ra:start_server` must be called with the same value. +%% Subset of ra_server:ra_server_config(). +%% Both `ra:add_member` and `ra:start_server` must be called with the same values. -type ra_new_server() :: #{id := ra_server_id(), %% If set, server will start as non-voter until later promoted by the %% leader. - non_voter_id => ra_nvid()}. + non_voter => boolean(), + uid => ra_uid()}. -type ra_peer_status() :: normal | {sending_snapshot, pid()} | suspended | disconnected. --type ra_voter_status() :: {voter, ra_voter_state()} | - {nonvoter, ra_voter_state()}. - --type ra_voter_state() :: #{nvid => ra_nvid(), - target => ra_index()}. +-type ra_voter_status() :: #{non_voter => boolean(), + uid => ra_uid(), + target => ra_index()}. -type ra_peer_state() :: #{next_index := non_neg_integer(), match_index := non_neg_integer(), diff --git a/src/ra_directory.erl b/src/ra_directory.erl index 1e42f88b..4b6e85c2 100644 --- a/src/ra_directory.erl +++ b/src/ra_directory.erl @@ -188,7 +188,7 @@ overview(System) when is_atom(System) -> pid => Pid, parent => Parent, state => S, - voter_status => V, + non_voter => V, cluster_name => ClusterName, snapshot_state => maps:get(UId, Snaps, undefined)}} diff --git a/src/ra_server.erl b/src/ra_server.erl index e40a039b..11326910 100644 --- a/src/ra_server.erl +++ b/src/ra_server.erl @@ -6,6 +6,8 @@ %% -module(ra_server). +-feature(maybe_expr, enable). + -include("ra.hrl"). -include("ra_server.hrl"). @@ -55,7 +57,7 @@ terminate/2, log_fold/3, log_read/2, - voter_status/1, + get_non_voter/1, recover/1 ]). @@ -73,7 +75,7 @@ log := term(), voted_for => 'maybe'(ra_server_id()), % persistent votes => non_neg_integer(), - voter_status => ra_voter_status(), + non_voter => boolean(), commit_index := ra_index(), last_applied := ra_index(), persisted_last_applied => ra_index(), @@ -197,7 +199,7 @@ max_pipeline_count => non_neg_integer(), ra_event_formatter => {module(), atom(), [term()]}, counter => counters:counters_ref(), - non_voter_id => ra_nvid(), + non_voter => boolean(), system_config => ra_system:config()}. -type mutable_config() :: #{cluster_name => ra_cluster_name(), @@ -300,17 +302,14 @@ init(#{id := Id, case ra_log:recover_snapshot(Log0) of undefined -> InitialMachineState = ra_machine:init(Machine, Name), - Clu = make_cluster(Id, InitialNodes, - #{voter_status => init_voter_status(Config)}), - {0, Clu, + {0, make_cluster(Id, InitialNodes), 0, InitialMachineState, {0, 0}}; {#{index := Idx, term := Term, cluster := ClusterNodes0, machine_version := MacVersion} = Snapshot, MacSt} -> ClusterNodes = maps:get(cluster_state, Snapshot, ClusterNodes0), - Clu = make_cluster(Id, ClusterNodes, - #{voter_status => init_voter_status(Config)}), + Clu = make_cluster(Id, ClusterNodes), %% the snapshot is the last index before the first index %% TODO: should this be Idx + 1? {Idx + 1, Clu, MacVersion, MacSt, {Idx, Term}} @@ -335,6 +334,9 @@ init(#{id := Id, put_counter(Cfg, ?C_RA_SVR_METRIC_LAST_APPLIED, SnapshotIdx), put_counter(Cfg, ?C_RA_SVR_METRIC_TERM, CurrentTerm), + NonVoter = get_non_voter(Cluster0, Id, UId, + maps:get(non_voter, Config, false)), + #{cfg => Cfg, current_term => CurrentTerm, cluster => Cluster0, @@ -345,7 +347,7 @@ init(#{id := Id, cluster_change_permitted => false, cluster_index_term => SnapshotIndexTerm, voted_for => VotedFor, - voter_status => voter_status(Id, Cluster0), + non_voter => NonVoter, commit_index => CommitIndex, %% set this to the first index so that we can apply all entries %% up to the commit index during recovery @@ -1119,16 +1121,18 @@ handle_follower({ra_log_event, Evt}, State = #{log := Log0}) -> {Log, Effects} = ra_log:handle_event(Evt, Log0), {follower, State#{log => Log}, Effects}; handle_follower(#pre_vote_rpc{}, - #{cfg := #cfg{log_id = LogId}, voter_status := {nonvoter, _} = Voter} = State) -> + #{cfg := #cfg{log_id = LogId}, + voter_status := #{non_voter := true} = VoterStatus} = State) -> ?DEBUG("~s: follower ignored pre_vote_rpc, non-voter: ~p0", - [LogId, Voter]), + [LogId, VoterStatus]), {follower, State, []}; handle_follower(#pre_vote_rpc{} = PreVote, State) -> process_pre_vote(follower, PreVote, State); handle_follower(#request_vote_rpc{}, - #{cfg := #cfg{log_id = LogId}, voter_status := {nonvoter, _} = Voter} = State) -> + #{cfg := #cfg{log_id = LogId}, + voter_status := #{non_voter := true} = VoterStatus} = State) -> ?DEBUG("~s: follower ignored request_vote_rpc, non-voter: ~p0", - [LogId, Voter]), + [LogId, VoterStatus]), {follower, State, []}; handle_follower(#request_vote_rpc{candidate_id = Cand, term = Term}, #{current_term := Term, voted_for := VotedFor, @@ -1228,9 +1232,9 @@ handle_follower(#append_entries_reply{}, State) -> {follower, State, []}; handle_follower(election_timeout, #{cfg := #cfg{log_id = LogId}, - voter_status := {nonvoter, _} = Voter} = State) -> + voter_status := #{non_voter := true} = VoterStatus} = State) -> ?DEBUG("~s: follower ignored election_timeout, non-voter: ~p0", - [LogId, Voter]), + [LogId, VoterStatus]), {follower, State, []}; handle_follower(election_timeout, State) -> call_for_election(pre_vote, State); @@ -1293,13 +1297,14 @@ handle_receive_snapshot(#install_snapshot_rpc{term = Term, {#{cluster := ClusterIds0} = Snapshot, MacState} = ra_log:recover_snapshot(Log), ClusterIds = maps:get(cluster_state, Snapshot, ClusterIds0), + Cluster = make_cluster(Id, ClusterIds), State = update_term(Term, State0#{cfg => Cfg, log => Log, commit_index => SnapIndex, last_applied => SnapIndex, - cluster => make_cluster(Id, ClusterIds, - #{voter_status => init_voter_status(State0)}), + cluster => Cluster, + non_voter => get_non_voter(Cluster, State0), machine_state => MacState}), %% it was the last snapshot chunk so we can revert back to %% follower status @@ -1402,12 +1407,12 @@ overview(#{cfg := #cfg{effective_machine_module = MacMod} = Cfg, cluster, leader_id, voted_for, + non_voter, cluster_change_permitted, cluster_index_term, query_index ], State), - O1 = O0#{voter_status => voter_status(State)}, - O = maps:merge(O1, cfg_to_map(Cfg)), + O = maps:merge(O0, cfg_to_map(Cfg)), LogOverview = ra_log:overview(Log), MacOverview = ra_machine:overview(MacMod, MacState), O#{log => LogOverview, @@ -2119,7 +2124,6 @@ new_peer() -> match_index => 0, commit_index_sent => 0, query_index => 0, - voter_status => {voter, #{}}, status => normal}. new_peer_with(Map) -> @@ -2240,37 +2244,34 @@ fetch_term(Idx, #{log := Log0} = State) -> {Term, State#{log => Log}} end. -make_cluster(Self, Nodes0, DefaultSelf) when is_list(Nodes0) -> +make_cluster(Self, Nodes0) when is_list(Nodes0) -> Nodes = lists:foldl(fun(N, Acc) -> Acc#{N => new_peer()} end, #{}, Nodes0), - make_cluster0(Self, Nodes, DefaultSelf); -make_cluster(Self, Nodes0, DefaultSelf) when is_map(Nodes0) -> - Nodes = maps:map(fun(_, V0) -> - V1 = maps:with([voter_status], V0), - maps:merge(new_peer(), V1) + append_self(Self, Nodes); +make_cluster(Self, Nodes0) when is_map(Nodes0) -> + Nodes = maps:map(fun(_, Peer0) -> + Peer1 = maps:with([voter_status], Peer0), + new_peer_with(Peer1) end, Nodes0), - make_cluster0(Self, Nodes, DefaultSelf). + append_self(Self, Nodes). -make_cluster0(Self, Nodes, DefaultSelf) -> +append_self(Self, Nodes) -> case Nodes of #{Self := _} = Cluster -> % current server is already in cluster - do nothing Cluster; Cluster -> % add current server to cluster - Cluster#{Self => maps:merge(new_peer(), DefaultSelf)} + Cluster#{Self => new_peer()} end. -initialise_peers(State = #{cfg := #cfg{id = Id}, log := Log, cluster := Cluster0}) -> +initialise_peers(State = #{log := Log, cluster := Cluster0}) -> NextIdx = ra_log:next_index(Log), - Cluster = maps:map(fun (PeerId, Self) when PeerId =:= Id -> - Self; - (_, Peer) -> - %% if key not present we assume `voter' status - Voter = maps:get(voter_status, Peer, {voter, #{}}), - new_peer_with(#{next_index => NextIdx, - voter_status => Voter}) + Cluster = maps:map(fun (_, Peer) -> + Peer1 = maps:with([voter_status], Peer), + Peer2 = Peer1#{next_index => NextIdx}, + new_peer_with(Peer2) end, Cluster0), State#{cluster => Cluster}. @@ -2367,7 +2368,7 @@ apply_with({Idx, Term, {'$ra_cluster_change', CmdMeta, NewCluster, ReplyType}}, [log_id(State0), maps:keys(NewCluster)]), %% we are recovering and should apply the cluster change State0#{cluster => NewCluster, - voter_status => maybe_promote_self(NewCluster, State0), + non_voter => get_non_voter(NewCluster, State0), cluster_change_permitted => true, cluster_index_term => {Idx, Term}}; _ -> @@ -2521,10 +2522,12 @@ append_log_leader({'$ra_join', From, append_cluster_change(Cluster, From, ReplyMode, State) end end; -append_log_leader({'$ra_join', From, #{id := JoiningNode} = Spec, ReplyMode}, +append_log_leader({'$ra_join', From, #{id := JoiningNode} = Config, ReplyMode}, State) -> append_log_leader({'$ra_join', From, - #{id => JoiningNode, voter_status => init_voter_status(Spec)}, + #{id => JoiningNode, + voter_status => maps:with([non_voter, uid, target], + Config)}, ReplyMode}, State); append_log_leader({'$ra_join', From, JoiningNode, ReplyMode}, State = #{cluster := OldCluster}) -> @@ -2575,7 +2578,7 @@ pre_append_log_follower({Idx, Term, Cmd} = Entry, pre_append_log_follower({Idx, Term, {'$ra_cluster_change', _, Cluster, _}}, State) -> State#{cluster => Cluster, - voter_status => maybe_promote_self(Cluster, State), + non_voter => get_non_voter(Cluster, State), cluster_index_term => {Idx, Term}}; pre_append_log_follower(_, State) -> State. @@ -2654,7 +2657,7 @@ query_indexes(#{cfg := #cfg{id = Id}, query_index := QueryIndex}) -> maps:fold(fun (PeerId, _, Acc) when PeerId == Id -> Acc; - (_K, #{voter_status := {nonvoter, _}}, Acc) -> + (_K, #{voter_status := #{non_voter := true}}, Acc) -> Acc; (_K, #{query_index := Idx}, Acc) -> [Idx | Acc] @@ -2666,7 +2669,7 @@ match_indexes(#{cfg := #cfg{id = Id}, {LWIdx, _} = ra_log:last_written(Log), maps:fold(fun (PeerId, _, Acc) when PeerId == Id -> Acc; - (_K, #{voter_status := {nonvoter, _}}, Acc) -> + (_K, #{voter_status := #{non_voter := true}}, Acc) -> Acc; (_K, #{match_index := Idx}, Acc) -> [Idx | Acc] @@ -2901,67 +2904,58 @@ already_member(State) -> -spec ensure_promotion_target(ra_voter_status(), ra_index()) -> {ok, ra_voter_status()} | {error, term()}. -ensure_promotion_target({voter, Reason}, _) -> - {ok, {voter, Reason}}; -ensure_promotion_target({nonvoter, #{target := _, nvid := _} = Reason}, _) -> - {ok, {nonvoter, Reason}}; -ensure_promotion_target({nonvoter, #{nvid := _} = Reason}, +ensure_promotion_target(#{non_voter := true, target := _, uid := _} = Status, + _) -> + {ok, Status}; +ensure_promotion_target(#{non_voter := true, uid := _} = Status, #{log := Log}) -> Target = ra_log:next_index(Log), - {ok, {nonvoter, Reason#{target => Target}}}; -ensure_promotion_target(_, _) -> - {error, missing_nvid}. - --spec init_voter_status(ra_server_config() | ra_new_server()) -> ra_voter_status(). -init_voter_status(#{non_voter_id := NVId}) -> - {nonvoter, #{nvid => NVId}}; -init_voter_status(_) -> - {voter, #{}}. - --spec voter_status(ra_server_state()) -> ra_voter_status(). -voter_status(#{cluster := Cluster} = State) -> - case maps:get(voter_status, State, undefined) of - undefined -> - voter_status(id(State), Cluster); - Voter -> - Voter - end. - --spec voter_status(ra_server_id(), ra_cluster()) -> ra_voter_status(). -voter_status(PeerId, Cluster) -> + {ok, Status#{target => Target}}; +ensure_promotion_target(#{non_voter := true}, _) -> + {error, missing_uid}; +ensure_promotion_target(Voter, _) -> + {ok, Voter}. + +%% Get non_voter of a given Id+UId from a (possibly new) cluster. +-spec get_non_voter(ra_cluster(), ra_server_id(), ra_uid(), boolean()) -> boolean(). +get_non_voter(Cluster, PeerId, UId, Default) -> case maps:get(PeerId, Cluster, undefined) of - undefined -> - {undefined, #{}}; - Peer -> - maps:get(voter_status, Peer, {voter, #{}}) + #{voter_status := #{uid := UId} = VoterStatus} -> + maps:get(non_voter, VoterStatus, Default); + _ -> + Default end. --spec maybe_promote_self(ra_cluster(), ra_server_state()) -> ra_voter_status(). -maybe_promote_self(NewCluster, State) -> - %% A bit complicated procedure since nvid is not guaranteed. - {_, NReason} = New = voter_status(id(State), NewCluster), - {_, CReason} = Current = voter_status(State), - Self = maps:get(nvid, CReason, undefined), - case maps:get(nvid, NReason, undefined) of - Self -> New; - _ -> Current - end. +%% Get this node's non_voter from a (possibly new) cluster. +%% Defaults to last known-locally value. +-spec get_non_voter(ra_cluster(), ra_state()) -> boolean(). +get_non_voter(Cluster, #{cfg := #cfg{id = Id, uid = UId}} = State) -> + Default = maps:get(non_voter, State, false), + get_non_voter(Cluster, Id, UId, Default). + +%% Get this node's non_voter. +%% Defaults to last known-locally value. +-spec get_non_voter(ra_state()) -> boolean(). +get_non_voter(#{cfg := #cfg{id = Id, uid = UId}, cluster := Cluster} = State) -> + Default = maps:get(non_voter, State, false), + get_non_voter(Cluster, Id, UId, Default). -spec maybe_promote_peer(ra_server_id(), ra_server_state(), effects()) -> effects(). maybe_promote_peer(PeerID, #{cluster := Cluster}, Effects) -> - % Unknown peer handled in the caller. - #{PeerID := #{match_index := MI, - voter_status := Status}} = Cluster, - case Status of - {nonvoter, #{target := Target} = Reason} when MI >= Target -> - [{next_event, - {command, {'$ra_join', - #{ts => os:system_time(millisecond)}, - #{id => PeerID, voter_status => {voter, Reason}}, - noreply}}} | - Effects]; - _ -> - Effects + maybe + #{PeerID := #{match_index := MI, + voter_status := VoterStatus}} ?= Cluster, + #{non_voter := true, target := Target} ?= VoterStatus, + true ?= MI >= Target, + E = {next_event, + {command, {'$ra_join', + #{ts => os:system_time(millisecond)}, + #{id => PeerID, + voter_status => VoterStatus#{non_voter => false}}, + noreply}}}, + [E | Effects] + else + _ -> Effects end. -spec required_quorum(ra_cluster()) -> pos_integer(). @@ -2971,7 +2965,7 @@ required_quorum(Cluster) -> count_voters(Cluster) -> maps:fold( - fun (_, #{voter_status := {nonvoter, _}}, Count) -> + fun (_, #{voter_status := #{non_voter := true}}, Count) -> Count; (_, _, Count) -> Count + 1 diff --git a/src/ra_server_proc.erl b/src/ra_server_proc.erl index 6e4966b5..cda7699e 100644 --- a/src/ra_server_proc.erl +++ b/src/ra_server_proc.erl @@ -273,7 +273,7 @@ init(Config) -> do_init(#{id := Id, cluster_name := ClusterName} = Config0) -> Key = ra_lib:ra_server_id_to_local_name(Id), - true = ets:insert(ra_state, {Key, init, unknown}), + true = ets:insert(ra_state, {Key, init, false}), %% can't vote while initializing process_flag(trap_exit, true), Config = #{counter := Counter, system_config := SysConf} = maps:merge(config_defaults(Id), @@ -788,16 +788,16 @@ follower({call, From}, {log_fold, Fun, Term}, State) -> fold_log(From, Fun, Term, State); follower(EventType, Msg, #state{conf = #conf{name = Name}, server_state = SS0} = State0) -> - VoterStatus0 = ra_server:voter_status(SS0), + NonVoter0 = ra_server:get_non_voter(SS0), case handle_follower(Msg, State0) of {follower, State1, Effects} -> {State2, Actions} = ?HANDLE_EFFECTS(Effects, EventType, State1), State = #state{server_state = SS} = follower_leader_change(State0, State2), - case ra_server:voter_status(SS) of - VoterStatus0 -> + case ra_server:get_non_voter(SS) of + NonVoter0 -> ok; - VoterStatus -> - true = ets:update_element(ra_state, Name, {3, VoterStatus}) + NonVoter -> + true = ets:update_element(ra_state, Name, {3, NonVoter}) end, {keep_state, State, Actions}; {pre_vote, State1, Effects} -> @@ -1039,8 +1039,8 @@ format_status(Opt, [_PDict, StateName, handle_enter(RaftState, OldRaftState, #state{conf = #conf{name = Name}, server_state = ServerState0} = State) -> - VoterStatus = ra_server:voter_status(ServerState0), - true = ets:insert(ra_state, {Name, RaftState, VoterStatus}), + NonVoter = ra_server:get_non_voter(ServerState0), + true = ets:insert(ra_state, {Name, RaftState, NonVoter}), {ServerState, Effects} = ra_server:handle_state_enter(RaftState, ServerState0), case RaftState == leader orelse OldRaftState == leader of @@ -1523,8 +1523,10 @@ do_state_query(machine, #{machine_state := MacState}) -> MacState; do_state_query(voters, #{cluster := Cluster}) -> Voters = maps:filter(fun(_, Peer) -> - {Voter, _} = maps:get(voter_status, Peer, {voter, legacy}), - Voter == voter + case maps:get(voter_status, Peer, undefined) of + #{non_voter := true} -> false; + _ -> true + end end, Cluster), maps:keys(Voters); do_state_query(members, #{cluster := Cluster}) -> @@ -1736,15 +1738,15 @@ handle_tick_metrics(State) -> can_execute_locally(RaftState, TargetNode, #state{server_state = ServerState} = State) -> - {Voter, _} = ra_server:voter_status(ServerState), + NonVoter = ra_server:get_non_voter(ServerState), case RaftState of - follower when Voter == voter -> + follower when not NonVoter -> TargetNode == node(); leader when TargetNode =/= node() -> %% We need to evaluate whether to send the message. %% Only send if there isn't a local node for the target pid. - Voters = do_state_query(voters, State#state.server_state), - not lists:any(fun ({_, N}) -> N == TargetNode end, Voters); + Members = do_state_query(voters, State#state.server_state), + not lists:any(fun ({_, N}) -> N == TargetNode end, Members); leader -> true; _ -> diff --git a/test/coordination_SUITE.erl b/test/coordination_SUITE.erl index e0ac392c..9d8202d3 100644 --- a/test/coordination_SUITE.erl +++ b/test/coordination_SUITE.erl @@ -308,8 +308,9 @@ send_local_msg(Config) -> {ok, _, Leader} = ra:members(hd(NodeIds)), {ok, _, _} = ra:process_command(Leader, banana), New = #{id => NonVoter, - voter_status => {nonvoter, #{nvid => <<"test">>, target => 999}}, - non_voter_id => <<"test">>}, + voter_status => #{non_voter => true, uid => <<"test">>, target => 999}, + non_voter => true, + uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, NodeIds), %% select a non-leader node to spawn on @@ -346,8 +347,9 @@ local_log_effect(Config) -> {ok, _, Leader} = ra:members(hd(NodeIds)), {ok, _, _} = ra:process_command(Leader, banana), New = #{id => NonVoter, - voter_status => {nonvoter, #{nvid => <<"test">>, target => 999}}, - non_voter_id => <<"test">>}, + voter_status => #{non_voter => true, uid => <<"test">>, target => 999}, + non_voter => true, + uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, NodeIds), %% select a non-leader node to spawn on @@ -434,25 +436,24 @@ nonvoter_catches_up(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, non_voter_id => <<"test">>}, + New = #{id => C, non_voter => true, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), - NonVoter = {nonvoter, #{nvid => <<"test">>}}, - ?assertMatch(#{Group := #{voter_status := NonVoter}}, + ?assertMatch(#{Group := #{non_voter := true}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{voter_status := NonVoter}, + ?assertMatch(#{non_voter := true}, ra:key_metrics(C)), - ?assertMatch({ok, #{voter_status := NonVoter}, _}, + ?assertMatch({ok, #{non_voter := true}, _}, ra:member_overview(C)), await_condition( fun () -> - {ok, #{voter_status := {voter, #{nvid := <<"test">>}}}, _} = ra:member_overview(C), - true + {ok, #{non_voter := NV}, _} = ra:member_overview(C), + not NV end, 200), - ?assertMatch(#{Group := #{voter_status := {voter, #{nvid := <<"test">>}}}}, + ?assertMatch(#{Group := #{non_voter := false}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{voter_status := {voter, #{nvid := <<"test">>}}}, + ?assertMatch(#{non_voter := false}, ra:key_metrics(C)), [ok = slave:stop(S) || {_, S} <- ServerIds], @@ -470,27 +471,26 @@ nonvoter_catches_up_after_restart(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, non_voter_id => <<"test">>}, + New = #{id => C, non_voter => true, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), - NonVoter = {nonvoter, #{nvid => <<"test">>}}, - ?assertMatch(#{Group := #{voter_status := NonVoter}}, + ?assertMatch(#{Group := #{non_voter := true}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{voter_status := NonVoter}, + ?assertMatch(#{non_voter := true}, ra:key_metrics(C)), - ?assertMatch({ok, #{voter_status := NonVoter}, _}, + ?assertMatch({ok, #{non_voter := true}, _}, ra:member_overview(C)), ok = ra:stop_server(?SYS, C), ok = ra:restart_server(?SYS, C), await_condition( fun () -> - {ok, #{voter_status := {voter, #{nvid := <<"test">>}}}, _} = ra:member_overview(C), - true + {ok, #{non_voter := NV}, _} = ra:member_overview(C), + not NV end, 200), - ?assertMatch(#{Group := #{voter_status := {voter, #{nvid := <<"test">>}}}}, + ?assertMatch(#{Group := #{non_voter := false}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{voter_status := {voter, #{nvid := <<"test">>}}}, + ?assertMatch(#{non_voter := false}, ra:key_metrics(C)), [ok = slave:stop(S) || {_, S} <- ServerIds], @@ -508,27 +508,26 @@ nonvoter_catches_up_after_leader_restart(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, non_voter_id => <<"test">>}, + New = #{id => C, non_voter => true, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), - NonVoter = {nonvoter, #{nvid => <<"test">>}}, - ?assertMatch(#{Group := #{voter_status := NonVoter}}, + ?assertMatch(#{Group := #{non_voter := true}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{voter_status := NonVoter}, + ?assertMatch(#{non_voter := true}, ra:key_metrics(C)), - ?assertMatch({ok, #{voter_status := NonVoter}, _}, + ?assertMatch({ok, #{non_voter := true}, _}, ra:member_overview(C)), ok = ra:stop_server(?SYS, Leader), ok = ra:restart_server(?SYS, Leader), await_condition( fun () -> - {ok, #{voter_status := {voter, #{nvid := <<"test">>}}}, _} = ra:member_overview(C), - true + {ok, #{non_voter := NV}, _} = ra:member_overview(C), + not NV end, 200), - ?assertMatch(#{Group := #{voter_status := {voter, #{nvid := <<"test">>}}}}, + ?assertMatch(#{Group := #{non_voter := false}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{voter_status := {voter, #{nvid := <<"test">>}}}, + ?assertMatch(#{non_voter := false}, ra:key_metrics(C)), [ok = slave:stop(S) || {_, S} <- ServerIds], diff --git a/test/ra_2_SUITE.erl b/test/ra_2_SUITE.erl index 4fb8bdbc..f5ff9a1c 100644 --- a/test/ra_2_SUITE.erl +++ b/test/ra_2_SUITE.erl @@ -674,6 +674,7 @@ force_start_follower_as_single_member(Config) -> UId4 = ?config(uid4, Config), Conf4 = conf(ClusterName, UId4, ServerId4, PrivDir, [ServerId3]), {ok, _, _} = ra:add_member(ServerId3, ServerId4), + %% the membership has changed but member not running yet {timeout,_} = ra:process_command(ServerId3, {enq, banana}), %% start new member ok = ra:start_server(?SYS, Conf4), @@ -713,12 +714,12 @@ force_start_follower_as_single_member_nonvoter(Config) -> ServerId4 = ?config(server_id4, Config), UId4 = ?config(uid4, Config), Conf4 = conf(ClusterName, UId4, ServerId4, PrivDir, [ServerId3]), - {ok, _, _} = ra:add_member(ServerId3, #{id => ServerId4, non_voter_id => <<"test">>}), + {ok, _, _} = ra:add_member(ServerId3, #{id => ServerId4, non_voter => true, uid => <<"test">>}), %% the membership has changed but member not running yet %% it is nonvoter and does not affect quorum size {ok, _, _} = ra:process_command(ServerId3, {enq, banana}), %% start new member - ok = ra:start_server(?SYS, Conf4#{non_voter_id => <<"test">>}), + ok = ra:start_server(?SYS, Conf4#{non_voter=> true, uid => <<"test">>}), {ok, _, ServerId3} = ra:members(ServerId4), ok = enqueue(ServerId3, msg3), diff --git a/test/ra_SUITE.erl b/test/ra_SUITE.erl index d3481339..7177e0d3 100644 --- a/test/ra_SUITE.erl +++ b/test/ra_SUITE.erl @@ -1039,13 +1039,15 @@ new_nonvoter_knows_its_status(Config) -> % n2 had no time to catch up % in server state - {ok, #{cluster := #{N1 := #{voter_status := {voter, _}}, - N2 := #{voter_status := {nonvoter, #{target := 2, nvid := T}}}}}, _} = ra:member_overview(N1), - {ok, #{cluster := #{N1 := #{voter_status := {voter, _}}, - N2 := #{voter_status := {nonvoter, #{nvid := T}}}}}, _} = ra:member_overview(N2), + {ok, #{uid := T, non_voter := true}, _} = ra:member_overview(N2), + {ok, + #{cluster := #{N2 := #{voter_status := #{non_voter := true, + target := 3, + uid := T}}}}, + _} = ra:member_overview(N1), % in ets - #{servers := #{n1 := #{voter_status := {voter, _}}, - n2 := #{voter_status := {nonvoter, _}}}} = ra:overview(?SYS), + #{servers := #{n1 := #{non_voter := false}, + n2 := #{non_voter := true}}} = ra:overview(?SYS), ok. voter_gets_promoted_consistent_leader(Config) -> @@ -1074,7 +1076,7 @@ voter_gets_promoted_consistent_leader(Config) -> lists:map(fun(O) -> ?assertEqual(All, voters(O)) end, overviews(N1)), % in ets #{servers := Servers} = ra:overview(?SYS), - lists:map(fun({Name, _}) -> #{Name := #{voter_status := {voter, _}}} = Servers end, All), + lists:map(fun({Name, _}) -> #{Name := #{non_voter := false}} = Servers end, All), ok. voter_gets_promoted_new_leader(Config) -> @@ -1099,7 +1101,7 @@ voter_gets_promoted_new_leader(Config) -> lists:map(fun(O) -> ?assertEqual(All, voters(O)) end, overviews(N1)), % in ets #{servers := Servers} = ra:overview(?SYS), - lists:map(fun({Name, _}) -> #{Name := #{voter_status := {voter, _}}} = Servers end, All), + lists:map(fun({Name, _}) -> #{Name := #{non_voter := false}} = Servers end, All), ok. get_gen_statem_status(Ref) -> @@ -1175,7 +1177,8 @@ start_and_join({ClusterName, _} = ServerRef, {_, _} = New) -> ok. start_and_join_nonvoter({ClusterName, _} = ServerRef, {_, _} = New) -> - Server = #{id => New, non_voter_id => <<"test">>}, + UId = ra:new_uid(ra_lib:to_binary(ClusterName)), + Server = #{id => New, non_voter => true, uid => UId}, {ok, _, _} = ra:add_member(ServerRef, Server), ok = ra:start_server(default, ClusterName, Server, add_machine(), [ServerRef]), ok. @@ -1215,10 +1218,7 @@ overviews(Node) -> [ra:member_overview(P) || {_, _} = P <- Members]. voters({ok, #{cluster := Peers}, _} = _Overview) -> - [Id || {Id, Status} <- maps:to_list(Peers), begin - {Voter, _} = maps:get(voter_status, Status), - Voter == voter - end]. + [Id || {Id, Status} <- maps:to_list(Peers), not maps:get(non_voter, Status, false)]. %% machine impl init(_) -> 0. diff --git a/test/ra_log_snapshot_SUITE.erl b/test/ra_log_snapshot_SUITE.erl index cd401a78..0fa3fd66 100644 --- a/test/ra_log_snapshot_SUITE.erl +++ b/test/ra_log_snapshot_SUITE.erl @@ -198,5 +198,4 @@ meta(Idx, Term, Cluster) -> #{index => Idx, term => Term, cluster => Cluster, - non_voters => maps:from_list([{N, #{}} || N <- Cluster]), machine_version => 1}. diff --git a/test/ra_server_SUITE.erl b/test/ra_server_SUITE.erl index dcf7604d..c435c221 100644 --- a/test/ra_server_SUITE.erl +++ b/test/ra_server_SUITE.erl @@ -280,7 +280,7 @@ election_timeout(_Config) -> ra_server:handle_follower(Msg, State), % non-voters ignore election_timeout - NVState = State#{voter_status => {nonvoter, test}}, + NVState = State#{voter_status => #{non_voter => true}}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), % pre_vote @@ -806,19 +806,19 @@ candidate_handles_append_entries_rpc(_Config) -> append_entries_reply_success_promotes_nonvoter(_Config) -> N1 = ?N1, N2 = ?N2, N3 = ?N3, - NonVoter = {nonvoter, #{target => 3, nvid => <<"test">>}}, + NonVoter = #{non_voter => true, target => 3, uid => <<"uid">>}, Cluster = #{N1 => new_peer_with(#{next_index => 5, match_index => 4}), N2 => new_peer_with(#{next_index => 1, match_index => 0, commit_index_sent => 3, voter_status => NonVoter}), N3 => new_peer_with(#{next_index => 2, match_index => 1})}, State0 = (base_state(3, ?FUNCTION_NAME))#{commit_index => 1, - last_applied => 1, - cluster => Cluster, - machine_state => <<"hi1">>}, + last_applied => 1, + cluster => Cluster, + machine_state => <<"hi1">>}, Ack = #append_entries_reply{term = 5, success = true, - next_index = 4, - last_index = 3, last_term = 5}, + next_index = 4, + last_index = 3, last_term = 5}, % doesn't progress commit_index, non voter ack doesn't raise majority {leader, #{cluster := #{N2 := #{next_index := 4, @@ -829,7 +829,9 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> machine_state := <<"hi1">>} = State1, [{next_event, info, pipeline_rpcs}, {next_event, {command, {'$ra_join', _, - #{id := N2, voter_status := {voter, _}}, noreply}} = RaJoin} + #{id := N2, + voter_status := #{non_voter := false, uid := <<"uid">>}}, + noreply}} = RaJoin} ]} = ra_server:handle_leader({N2, Ack}, State0), % pipeline to N3 @@ -850,7 +852,7 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> % ra_join translates into cluster update {leader, #{cluster := #{N2 := #{next_index := 5, match_index := 3, - voter_status := {voter, _}}}, + voter_status := #{non_voter := false}}}, cluster_change_permitted := false, commit_index := 1, last_applied := 1, @@ -861,7 +863,7 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> prev_log_term = 5, leader_commit = 1, entries = [{4, 5, {'$ra_cluster_change', _, - #{N2 := #{voter_status := {voter, _}}}, + #{N2 := #{voter_status := #{non_voter := false}}}, _}}]}}, {send_rpc, N2, #append_entries_rpc{term = 5, leader_id = N1, @@ -869,7 +871,7 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> prev_log_term = 5, leader_commit = 1, entries = [{4, 5, {'$ra_cluster_change', _, - #{N2 := #{voter_status := {voter, _}}}, + #{N2 := #{voter_status := #{non_voter := false}}}, _}}]}} ]} = ra_server:handle_leader(RaJoin, State2), @@ -1011,7 +1013,7 @@ follower_request_vote(_Config) -> State), % non-voters ignore request_vote_rpc - NVState = State#{voter_status => {nonvoter, test}}, + NVState = State#{voter_status => #{non_voter => true}}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), ok. @@ -1130,7 +1132,7 @@ follower_pre_vote(_Config) -> State), % non-voters ignore pre_vote_rpc - NVState = State#{voter_status => {nonvoter, test}}, + NVState = State#{voter_status => #{non_voter => true}}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), ok. @@ -1406,18 +1408,17 @@ leader_server_join(_Config) -> cluster_change_permitted := false} = _State1, Effects} = ra_server:handle_leader({command, {'$ra_join', meta(), N4, await_consensus}}, State0), - % new member should join as voter [ {send_rpc, N4, #append_entries_rpc{entries = [_, _, _, {4, 5, {'$ra_cluster_change', _, #{N1 := _, N2 := _, - N3 := _, N4 := #{voter_status := {voter, _}}}, + N3 := _, N4 := N4Peer}, await_consensus}}]}}, {send_rpc, N3, #append_entries_rpc{entries = [{4, 5, {'$ra_cluster_change', _, - #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := {voter, _}}}, + #{N1 := _, N2 := _, N3 := _, N4 := N4Peer}, await_consensus}}], term = 5, leader_id = N1, prev_log_index = 3, @@ -1426,13 +1427,14 @@ leader_server_join(_Config) -> {send_rpc, N2, #append_entries_rpc{entries = [{4, 5, {'$ra_cluster_change', _, - #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := {voter, _}}}, + #{N1 := _, N2 := _, N3 := _, N4 := N4Peer}, await_consensus}}], term = 5, leader_id = N1, prev_log_index = 3, prev_log_term = 5, leader_commit = 3}} | _] = Effects, + undefined = maps:get(non_voter, N4Peer, undefined), ok. leader_server_join_nonvoter(_Config) -> @@ -1440,26 +1442,26 @@ leader_server_join_nonvoter(_Config) -> OldCluster = #{N1 => new_peer_with(#{next_index => 4, match_index => 3}), N2 => new_peer_with(#{next_index => 4, match_index => 3}), N3 => new_peer_with(#{next_index => 4, match_index => 3})}, - State0 = (base_state(3, ?FUNCTION_NAME))#{cluster => OldCluster}, + #{log := Log} = State0 = (base_state(3, ?FUNCTION_NAME))#{cluster => OldCluster}, % raft servers should switch to the new configuration after log append % and further cluster changes should be disallowed {leader, #{cluster := #{N1 := _, N2 := _, N3 := _, N4 := _}, - commit_index := Target, cluster_change_permitted := false} = _State1, Effects} = ra_server:handle_leader({command, {'$ra_join', meta(), - #{id => N4, non_voter_id => <<"test">>}, await_consensus}}, State0), + #{id => N4, non_voter => true, uid => <<"uid">>}, await_consensus}}, State0), % new member should join as non-voter + Status = #{non_voter => true, uid => <<"uid">>, target => ra_log:next_index(Log)}, [ {send_rpc, N4, #append_entries_rpc{entries = [_, _, _, {4, 5, {'$ra_cluster_change', _, #{N1 := _, N2 := _, - N3 := _, N4 := #{voter_status := {nonvoter, #{target := Target}}}}, + N3 := _, N4 := #{voter_status := Status}}, await_consensus}}]}}, {send_rpc, N3, #append_entries_rpc{entries = [{4, 5, {'$ra_cluster_change', _, - #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := {nonvoter, #{target := Target}}}}, + #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := Status}}, await_consensus}}], term = 5, leader_id = N1, prev_log_index = 3, @@ -1468,7 +1470,7 @@ leader_server_join_nonvoter(_Config) -> {send_rpc, N2, #append_entries_rpc{entries = [{4, 5, {'$ra_cluster_change', _, - #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := {nonvoter, #{target := Target}}}}, + #{N1 := _, N2 := _, N3 := _, N4 := #{voter_status := Status}}, await_consensus}}], term = 5, leader_id = N1, prev_log_index = 3, @@ -1581,12 +1583,13 @@ leader_applies_new_cluster(_Config) -> ra_server:handle_leader(written_evt({4, 4, 5}), State1), ?assert(not maps:get(cluster_change_permitted, State2)), - + % replies coming in AEReply = #append_entries_reply{term = 5, success = true, next_index = 5, last_index = 4, last_term = 5}, % leader does not yet have consensus as will need at least 3 votes {leader, State3 = #{commit_index := 3, + cluster_change_permitted := false, cluster_index_term := {4, 5}, cluster := #{N2 := #{next_index := 5, @@ -1608,12 +1611,13 @@ leader_applies_new_cluster_nonvoter(_Config) -> N3 => new_peer_with(#{next_index => 4, match_index => 3})}, State = (base_state(3, ?FUNCTION_NAME))#{cluster => OldCluster}, - Command = {command, {'$ra_join', meta(), #{id => N4, non_voter_id => <<"test">>}, await_consensus}}, + Command = {command, {'$ra_join', meta(), #{id => N4, non_voter => true, uid => <<"uid">>}, await_consensus}}, % cluster records index and term it was applied to determine whether it has % been applied {leader, #{cluster_index_term := {4, 5}, cluster := #{N1 := _, N2 := _, - N3 := _, N4 := _} } = State1, _} = + N3 := _, N4 := #{voter_status := #{non_voter := true, + uid := <<"uid">>}}}} = State1, _} = ra_server:handle_leader(Command, State), {leader, State2, _} = ra_server:handle_leader(written_evt({4, 4, 5}), State1), @@ -2125,23 +2129,23 @@ leader_received_append_entries_reply_and_promotes_voter(_config) -> last_index = 4, last_term = 5}, % Permanent voter - State1 = set_peer_voter_status(State, N3, {voter, #{nvid => <<"test">>}}), + State1 = set_peer_voter_status(State, N3, #{non_voter => false}), {leader, _, [{next_event,info,pipeline_rpcs}] } = ra_server:handle_leader({N3, AER}, State1), % Permanent non-voter - State2 = set_peer_voter_status(State, N3, {nonvoter, test}), + State2 = set_peer_voter_status(State, N3, #{non_voter => true}), {leader, _, [{next_event,info,pipeline_rpcs}] } = ra_server:handle_leader({N3, AER}, State2), % Promotion State3 = set_peer_voter_status(State, N3, - {nonvoter, #{target => 4}}), + #{non_voter => true, target => 4}), {leader, _, [{next_event,info,pipeline_rpcs}, - {next_event, {command, {'$ra_join', _, #{id := N3, voter_status := {voter, _}}, _}}}] + {next_event, {command, {'$ra_join', _, #{id := N3, voter_status := #{non_voter := false}}, _}}}] } = ra_server:handle_leader({N3, AER}, State3). follower_heartbeat(_Config) -> @@ -2793,7 +2797,6 @@ new_peer() -> match_index => 0, query_index => 0, commit_index_sent => 0, - voter_status => {voter, #{nvid => <<"test">>}}, status => normal}. new_peer_with(Map) -> From 831be889dfd72d1d176ad3e3de5d2adb72ba97ac Mon Sep 17 00:00:00 2001 From: Alex Valiushko Date: Wed, 20 Sep 2023 10:26:59 -0700 Subject: [PATCH 4/8] address feedback #2 --- src/ra.erl | 7 ++++--- src/ra.hrl | 10 ++++++---- src/ra_log.erl | 5 +++-- src/ra_server.erl | 34 ++++++++++++++++++++-------------- test/ra_snapshot_SUITE.erl | 29 +++++++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/ra.erl b/src/ra.erl index a7995aa9..f1750473 100644 --- a/src/ra.erl +++ b/src/ra.erl @@ -477,13 +477,14 @@ start_server(System, ClusterName, {_, _} = ServerId, Machine, ServerIds) -> start_server(System, ClusterName, #{id => ServerId}, Machine, ServerIds); start_server(System, ClusterName, #{id := {_, _}} = Conf0, Machine, ServerIds) when is_atom(System) -> - UId = new_uid(ra_lib:to_binary(ClusterName)), + UId = maps:get(uid, Conf0, + new_uid(ra_lib:to_binary(ClusterName))), Conf = #{cluster_name => ClusterName, uid => UId, initial_members => ServerIds, log_init_args => #{uid => UId}, machine => Machine}, - start_server(System, maps:merge(Conf, Conf0)). + start_server(System, maps:merge(Conf0, Conf)). %% @doc Starts a ra server in the default system %% @param Conf a ra_server_config() configuration map. @@ -1134,7 +1135,7 @@ key_metrics({Name, N} = ServerId) when N == node() -> case whereis(Name) of undefined -> Counters#{state => noproc, - non_voter => noproc}; + non_voter => unknown}; _ -> case ets:lookup(ra_state, Name) of [] -> diff --git a/src/ra.hrl b/src/ra.hrl index 7ad277d0..64c7022e 100644 --- a/src/ra.hrl +++ b/src/ra.hrl @@ -65,14 +65,17 @@ % used for evaluating pipeline status commit_index_sent := non_neg_integer(), %% whether the peer is part of the consensus - voter_status := ra_voter_status(), + voter_status => ra_voter_status(), %% indicates that a snapshot is being sent %% to the peer status := ra_peer_status()}. -type ra_cluster() :: #{ra_server_id() => ra_peer_state()}. --type ra_cluster_servers() :: [ra_server_id()]. +%% Dehydrated cluster: +-type ra_cluster_servers() :: [ra_server_id()]. % Deprecated +-type ra_peer_snapshot() :: #{voter_status => ra_voter_status()}. +-type ra_cluster_snapshot() :: #{ra_server_id() => ra_peer_snapshot()}. %% represent a unique entry in the ra log -type log_entry() :: {ra_index(), ra_term(), term()}. @@ -154,8 +157,7 @@ -type snapshot_meta() :: #{index := ra_index(), term := ra_term(), - cluster := ra_cluster_servers(), - cluster_state => ra_cluster(), %% TODO replace `cluster` + cluster := ra_cluster_snapshot(), machine_version := ra_machine:version()}. -record(install_snapshot_rpc, diff --git a/src/ra_log.erl b/src/ra_log.erl index 5bd897e8..421546de 100644 --- a/src/ra_log.erl +++ b/src/ra_log.erl @@ -637,14 +637,15 @@ update_release_cursor0(Idx, Cluster, MacVersion, MacState, #?MODULE{cfg = #cfg{snapshot_interval = SnapInter}, reader = Reader, snapshot_state = SnapState} = State0) -> - ClusterServerIds = maps:keys(Cluster), + ClusterServerIds = maps:map(fun (_, V) -> + maps:with([voter_status], V) + end, Cluster), SnapLimit = case ra_snapshot:current(SnapState) of undefined -> SnapInter; {I, _} -> I + SnapInter end, Meta = #{index => Idx, cluster => ClusterServerIds, - cluster_state => Cluster, machine_version => MacVersion}, % The release cursor index is the last entry _not_ contributing % to the current state. I.e. the last entry that can be discarded. diff --git a/src/ra_server.erl b/src/ra_server.erl index 11326910..b4ff0d58 100644 --- a/src/ra_server.erl +++ b/src/ra_server.erl @@ -306,9 +306,8 @@ init(#{id := Id, 0, InitialMachineState, {0, 0}}; {#{index := Idx, term := Term, - cluster := ClusterNodes0, - machine_version := MacVersion} = Snapshot, MacSt} -> - ClusterNodes = maps:get(cluster_state, Snapshot, ClusterNodes0), + cluster := ClusterNodes, + machine_version := MacVersion}, MacSt} -> Clu = make_cluster(Id, ClusterNodes), %% the snapshot is the last index before the first index %% TODO: should this be Idx + 1? @@ -1295,16 +1294,14 @@ handle_receive_snapshot(#install_snapshot_rpc{term = Term, Cfg0 end, - {#{cluster := ClusterIds0} = Snapshot, MacState} = ra_log:recover_snapshot(Log), - ClusterIds = maps:get(cluster_state, Snapshot, ClusterIds0), - Cluster = make_cluster(Id, ClusterIds), + {#{cluster := ClusterIds}, MacState} = ra_log:recover_snapshot(Log), State = update_term(Term, State0#{cfg => Cfg, log => Log, commit_index => SnapIndex, last_applied => SnapIndex, - cluster => Cluster, - non_voter => get_non_voter(Cluster, State0), + cluster => make_cluster(Id, ClusterIds), + non_voter => get_non_voter(ClusterIds, State0), machine_state => MacState}), %% it was the last snapshot chunk so we can revert back to %% follower status @@ -2244,15 +2241,17 @@ fetch_term(Idx, #{log := Log0} = State) -> {Term, State#{log => Log}} end. +-spec make_cluster(ra_server_id(), ra_cluster_snapshot() | [ra_server_id()]) -> + ra_cluster(). make_cluster(Self, Nodes0) when is_list(Nodes0) -> Nodes = lists:foldl(fun(N, Acc) -> Acc#{N => new_peer()} end, #{}, Nodes0), append_self(Self, Nodes); make_cluster(Self, Nodes0) when is_map(Nodes0) -> + ct:pal("MAKE CLUSTER ~p", [[Self, Nodes0]]), Nodes = maps:map(fun(_, Peer0) -> - Peer1 = maps:with([voter_status], Peer0), - new_peer_with(Peer1) + new_peer_with(Peer0) end, Nodes0), append_self(Self, Nodes). @@ -2268,8 +2267,8 @@ append_self(Self, Nodes) -> initialise_peers(State = #{log := Log, cluster := Cluster0}) -> NextIdx = ra_log:next_index(Log), - Cluster = maps:map(fun (_, Peer) -> - Peer1 = maps:with([voter_status], Peer), + Cluster = maps:map(fun (_, Peer0) -> + Peer1 = maps:with([voter_status], Peer0), Peer2 = Peer1#{next_index => NextIdx}, new_peer_with(Peer2) end, Cluster0), @@ -2917,7 +2916,12 @@ ensure_promotion_target(Voter, _) -> {ok, Voter}. %% Get non_voter of a given Id+UId from a (possibly new) cluster. --spec get_non_voter(ra_cluster(), ra_server_id(), ra_uid(), boolean()) -> boolean(). +-spec get_non_voter(ra_cluster() | ra_cluster_snapshot() | ra_cluster_servers(), + ra_server_id(), ra_uid(), boolean()) -> + boolean(). +get_non_voter(_Cluster, _PeerId, _UId, Default) when is_list(_Cluster) -> + %% Legacy cluster snapshot does not retain voter_status. + Default; get_non_voter(Cluster, PeerId, UId, Default) -> case maps:get(PeerId, Cluster, undefined) of #{voter_status := #{uid := UId} = VoterStatus} -> @@ -2928,7 +2932,9 @@ get_non_voter(Cluster, PeerId, UId, Default) -> %% Get this node's non_voter from a (possibly new) cluster. %% Defaults to last known-locally value. --spec get_non_voter(ra_cluster(), ra_state()) -> boolean(). +-spec get_non_voter(ra_cluster() | ra_cluster_snapshot() | ra_cluster_servers(), + ra_state()) -> + boolean(). get_non_voter(Cluster, #{cfg := #cfg{id = Id, uid = UId}} = State) -> Default = maps:get(non_voter, State, false), get_non_voter(Cluster, Id, UId, Default). diff --git a/test/ra_snapshot_SUITE.erl b/test/ra_snapshot_SUITE.erl index 36ff7847..32c74cd9 100644 --- a/test/ra_snapshot_SUITE.erl +++ b/test/ra_snapshot_SUITE.erl @@ -32,6 +32,7 @@ all_tests() -> take_snapshot, take_snapshot_crash, init_recover, + init_recover_voter_status, init_recover_multi_corrupt, init_recover_corrupt, read_snapshot, @@ -162,6 +163,34 @@ init_recover(Config) -> {ok, Meta, ?FUNCTION_NAME} = ra_snapshot:recover(Recover), ok. +init_recover_voter_status(Config) -> + UId = ?config(uid, Config), + State0 = ra_snapshot:init(UId, ra_log_snapshot, + ?config(snap_dir, Config), undefined), + Meta = meta(55, 2, #{node() => #{voter_status => test}}), + {State1, [{monitor, process, snapshot_writer, _}]} = + ra_snapshot:begin_snapshot(Meta, ?FUNCTION_NAME, State0), + receive + {ra_log_event, {snapshot_written, IdxTerm}} -> + _ = ra_snapshot:complete_snapshot(IdxTerm, State1), + ok + after 1000 -> + error(snapshot_event_timeout) + end, + + %% open a new snapshot state to simulate a restart + Recover = ra_snapshot:init(UId, ra_log_snapshot, + ?config(snap_dir, Config), undefined), + %% ensure last snapshot is recovered + %% it also needs to be validated as could have crashed mid write + undefined = ra_snapshot:pending(Recover), + {55, 2} = ra_snapshot:current(Recover), + 55 = ra_snapshot:last_index_for(UId), + + %% recover the meta data and machine state + {ok, Meta, ?FUNCTION_NAME} = ra_snapshot:recover(Recover), + ok. + init_multi(Config) -> UId = ?config(uid, Config), State0 = ra_snapshot:init(UId, ra_log_snapshot, From d70f25319e2a6c50d898e207b8bc637b9e4f90be Mon Sep 17 00:00:00 2001 From: Alex Valiushko Date: Wed, 20 Sep 2023 19:24:10 -0700 Subject: [PATCH 5/8] replace bool membership with enum --- src/ra.erl | 8 ++-- src/ra.hrl | 14 ++++--- src/ra_directory.erl | 2 +- src/ra_server.erl | 81 ++++++++++++++++++------------------- src/ra_server_proc.erl | 40 ++++++++++-------- test/coordination_SUITE.erl | 56 ++++++++++++------------- test/ra_2_SUITE.erl | 4 +- test/ra_SUITE.erl | 16 ++++---- test/ra_server_SUITE.erl | 34 ++++++++-------- 9 files changed, 131 insertions(+), 124 deletions(-) diff --git a/src/ra.erl b/src/ra.erl index f1750473..159628fc 100644 --- a/src/ra.erl +++ b/src/ra.erl @@ -1135,15 +1135,15 @@ key_metrics({Name, N} = ServerId) when N == node() -> case whereis(Name) of undefined -> Counters#{state => noproc, - non_voter => unknown}; + membership => unknown}; _ -> case ets:lookup(ra_state, Name) of [] -> Counters#{state => unknown, - non_voter => unknown}; - [{_, State, NonVoter}] -> + membership => unknown}; + [{_, State, Membership}] -> Counters#{state => State, - non_voter => NonVoter} + membership => Membership} end end; key_metrics({_, N} = ServerId) -> diff --git a/src/ra.hrl b/src/ra.hrl index 64c7022e..e48f10a8 100644 --- a/src/ra.hrl +++ b/src/ra.hrl @@ -43,10 +43,9 @@ %% Subset of ra_server:ra_server_config(). %% Both `ra:add_member` and `ra:start_server` must be called with the same values. -type ra_new_server() :: #{id := ra_server_id(), - - %% If set, server will start as non-voter until later promoted by the - %% leader. - non_voter => boolean(), + % Defaults to `voter` is absent. + membership => ra_membership(), + % Required for `promotable` in the above. uid => ra_uid()}. -type ra_peer_status() :: normal | @@ -54,7 +53,9 @@ suspended | disconnected. --type ra_voter_status() :: #{non_voter => boolean(), +-type ra_membership() :: voter | promotable | unknown. + +-type ra_voter_status() :: #{membership => ra_membership(), uid => ra_uid(), target => ra_index()}. @@ -64,7 +65,8 @@ % the commit index last sent % used for evaluating pipeline status commit_index_sent := non_neg_integer(), - %% whether the peer is part of the consensus + %% Whether the peer is part of the consensus. + %% Defaults to "yes" if absent. voter_status => ra_voter_status(), %% indicates that a snapshot is being sent %% to the peer diff --git a/src/ra_directory.erl b/src/ra_directory.erl index 4b6e85c2..de841475 100644 --- a/src/ra_directory.erl +++ b/src/ra_directory.erl @@ -188,7 +188,7 @@ overview(System) when is_atom(System) -> pid => Pid, parent => Parent, state => S, - non_voter => V, + membership => V, cluster_name => ClusterName, snapshot_state => maps:get(UId, Snaps, undefined)}} diff --git a/src/ra_server.erl b/src/ra_server.erl index b4ff0d58..967fad4a 100644 --- a/src/ra_server.erl +++ b/src/ra_server.erl @@ -57,7 +57,7 @@ terminate/2, log_fold/3, log_read/2, - get_non_voter/1, + get_membership/1, recover/1 ]). @@ -75,7 +75,7 @@ log := term(), voted_for => 'maybe'(ra_server_id()), % persistent votes => non_neg_integer(), - non_voter => boolean(), + membership => ra_membership(), commit_index := ra_index(), last_applied := ra_index(), persisted_last_applied => ra_index(), @@ -199,7 +199,7 @@ max_pipeline_count => non_neg_integer(), ra_event_formatter => {module(), atom(), [term()]}, counter => counters:counters_ref(), - non_voter => boolean(), + membership => ra_membership(), system_config => ra_system:config()}. -type mutable_config() :: #{cluster_name => ra_cluster_name(), @@ -333,8 +333,8 @@ init(#{id := Id, put_counter(Cfg, ?C_RA_SVR_METRIC_LAST_APPLIED, SnapshotIdx), put_counter(Cfg, ?C_RA_SVR_METRIC_TERM, CurrentTerm), - NonVoter = get_non_voter(Cluster0, Id, UId, - maps:get(non_voter, Config, false)), + NonVoter = get_membership(Cluster0, Id, UId, + maps:get(membership, Config, voter)), #{cfg => Cfg, current_term => CurrentTerm, @@ -346,7 +346,7 @@ init(#{id := Id, cluster_change_permitted => false, cluster_index_term => SnapshotIndexTerm, voted_for => VotedFor, - non_voter => NonVoter, + membership => NonVoter, commit_index => CommitIndex, %% set this to the first index so that we can apply all entries %% up to the commit index during recovery @@ -1121,7 +1121,7 @@ handle_follower({ra_log_event, Evt}, State = #{log := Log0}) -> {follower, State#{log => Log}, Effects}; handle_follower(#pre_vote_rpc{}, #{cfg := #cfg{log_id = LogId}, - voter_status := #{non_voter := true} = VoterStatus} = State) -> + voter_status := #{membership := promotable} = VoterStatus} = State) -> ?DEBUG("~s: follower ignored pre_vote_rpc, non-voter: ~p0", [LogId, VoterStatus]), {follower, State, []}; @@ -1129,7 +1129,7 @@ handle_follower(#pre_vote_rpc{} = PreVote, State) -> process_pre_vote(follower, PreVote, State); handle_follower(#request_vote_rpc{}, #{cfg := #cfg{log_id = LogId}, - voter_status := #{non_voter := true} = VoterStatus} = State) -> + voter_status := #{membership := promotable} = VoterStatus} = State) -> ?DEBUG("~s: follower ignored request_vote_rpc, non-voter: ~p0", [LogId, VoterStatus]), {follower, State, []}; @@ -1231,7 +1231,7 @@ handle_follower(#append_entries_reply{}, State) -> {follower, State, []}; handle_follower(election_timeout, #{cfg := #cfg{log_id = LogId}, - voter_status := #{non_voter := true} = VoterStatus} = State) -> + voter_status := #{membership := promotable} = VoterStatus} = State) -> ?DEBUG("~s: follower ignored election_timeout, non-voter: ~p0", [LogId, VoterStatus]), {follower, State, []}; @@ -1301,7 +1301,7 @@ handle_receive_snapshot(#install_snapshot_rpc{term = Term, commit_index => SnapIndex, last_applied => SnapIndex, cluster => make_cluster(Id, ClusterIds), - non_voter => get_non_voter(ClusterIds, State0), + membership => get_membership(ClusterIds, State0), machine_state => MacState}), %% it was the last snapshot chunk so we can revert back to %% follower status @@ -1404,7 +1404,7 @@ overview(#{cfg := #cfg{effective_machine_module = MacMod} = Cfg, cluster, leader_id, voted_for, - non_voter, + membership, cluster_change_permitted, cluster_index_term, query_index @@ -2249,7 +2249,6 @@ make_cluster(Self, Nodes0) when is_list(Nodes0) -> end, #{}, Nodes0), append_self(Self, Nodes); make_cluster(Self, Nodes0) when is_map(Nodes0) -> - ct:pal("MAKE CLUSTER ~p", [[Self, Nodes0]]), Nodes = maps:map(fun(_, Peer0) -> new_peer_with(Peer0) end, Nodes0), @@ -2367,7 +2366,7 @@ apply_with({Idx, Term, {'$ra_cluster_change', CmdMeta, NewCluster, ReplyType}}, [log_id(State0), maps:keys(NewCluster)]), %% we are recovering and should apply the cluster change State0#{cluster => NewCluster, - non_voter => get_non_voter(NewCluster, State0), + membership => get_membership(NewCluster, State0), cluster_change_permitted => true, cluster_index_term => {Idx, Term}}; _ -> @@ -2525,7 +2524,7 @@ append_log_leader({'$ra_join', From, #{id := JoiningNode} = Config, ReplyMode}, State) -> append_log_leader({'$ra_join', From, #{id => JoiningNode, - voter_status => maps:with([non_voter, uid, target], + voter_status => maps:with([membership, uid, target], Config)}, ReplyMode}, State); append_log_leader({'$ra_join', From, JoiningNode, ReplyMode}, @@ -2577,7 +2576,7 @@ pre_append_log_follower({Idx, Term, Cmd} = Entry, pre_append_log_follower({Idx, Term, {'$ra_cluster_change', _, Cluster, _}}, State) -> State#{cluster => Cluster, - non_voter => get_non_voter(Cluster, State), + membership => get_membership(Cluster, State), cluster_index_term => {Idx, Term}}; pre_append_log_follower(_, State) -> State. @@ -2656,7 +2655,7 @@ query_indexes(#{cfg := #cfg{id = Id}, query_index := QueryIndex}) -> maps:fold(fun (PeerId, _, Acc) when PeerId == Id -> Acc; - (_K, #{voter_status := #{non_voter := true}}, Acc) -> + (_K, #{voter_status := #{membership := promotable}}, Acc) -> Acc; (_K, #{query_index := Idx}, Acc) -> [Idx | Acc] @@ -2668,7 +2667,7 @@ match_indexes(#{cfg := #cfg{id = Id}, {LWIdx, _} = ra_log:last_written(Log), maps:fold(fun (PeerId, _, Acc) when PeerId == Id -> Acc; - (_K, #{voter_status := #{non_voter := true}}, Acc) -> + (_K, #{voter_status := #{membership := promotable}}, Acc) -> Acc; (_K, #{match_index := Idx}, Acc) -> [Idx | Acc] @@ -2903,61 +2902,61 @@ already_member(State) -> -spec ensure_promotion_target(ra_voter_status(), ra_index()) -> {ok, ra_voter_status()} | {error, term()}. -ensure_promotion_target(#{non_voter := true, target := _, uid := _} = Status, +ensure_promotion_target(#{membership := promotable, target := _, uid := _} = Status, _) -> {ok, Status}; -ensure_promotion_target(#{non_voter := true, uid := _} = Status, +ensure_promotion_target(#{membership := promotable, uid := _} = Status, #{log := Log}) -> Target = ra_log:next_index(Log), {ok, Status#{target => Target}}; -ensure_promotion_target(#{non_voter := true}, _) -> +ensure_promotion_target(#{membership := promotable}, _) -> {error, missing_uid}; ensure_promotion_target(Voter, _) -> {ok, Voter}. -%% Get non_voter of a given Id+UId from a (possibly new) cluster. --spec get_non_voter(ra_cluster() | ra_cluster_snapshot() | ra_cluster_servers(), +%% Get membership of a given Id+UId from a (possibly new) cluster. +-spec get_membership(ra_cluster() | ra_cluster_snapshot() | ra_cluster_servers(), ra_server_id(), ra_uid(), boolean()) -> boolean(). -get_non_voter(_Cluster, _PeerId, _UId, Default) when is_list(_Cluster) -> +get_membership(_Cluster, _PeerId, _UId, Default) when is_list(_Cluster) -> %% Legacy cluster snapshot does not retain voter_status. Default; -get_non_voter(Cluster, PeerId, UId, Default) -> +get_membership(Cluster, PeerId, UId, Default) -> case maps:get(PeerId, Cluster, undefined) of #{voter_status := #{uid := UId} = VoterStatus} -> - maps:get(non_voter, VoterStatus, Default); + maps:get(membership, VoterStatus, Default); _ -> Default end. -%% Get this node's non_voter from a (possibly new) cluster. +%% Get this node's membership from a (possibly new) cluster. %% Defaults to last known-locally value. --spec get_non_voter(ra_cluster() | ra_cluster_snapshot() | ra_cluster_servers(), +-spec get_membership(ra_cluster() | ra_cluster_snapshot() | ra_cluster_servers(), ra_state()) -> boolean(). -get_non_voter(Cluster, #{cfg := #cfg{id = Id, uid = UId}} = State) -> - Default = maps:get(non_voter, State, false), - get_non_voter(Cluster, Id, UId, Default). +get_membership(Cluster, #{cfg := #cfg{id = Id, uid = UId}} = State) -> + Default = maps:get(membership, State, voter), + get_membership(Cluster, Id, UId, Default). -%% Get this node's non_voter. +%% Get this node's membership. %% Defaults to last known-locally value. --spec get_non_voter(ra_state()) -> boolean(). -get_non_voter(#{cfg := #cfg{id = Id, uid = UId}, cluster := Cluster} = State) -> - Default = maps:get(non_voter, State, false), - get_non_voter(Cluster, Id, UId, Default). +-spec get_membership(ra_state()) -> boolean(). +get_membership(#{cfg := #cfg{id = Id, uid = UId}, cluster := Cluster} = State) -> + Default = maps:get(membership, State, voter), + get_membership(Cluster, Id, UId, Default). -spec maybe_promote_peer(ra_server_id(), ra_server_state(), effects()) -> effects(). -maybe_promote_peer(PeerID, #{cluster := Cluster}, Effects) -> +maybe_promote_peer(PeerId, #{cluster := Cluster}, Effects) -> maybe - #{PeerID := #{match_index := MI, + #{PeerId := #{match_index := MI, voter_status := VoterStatus}} ?= Cluster, - #{non_voter := true, target := Target} ?= VoterStatus, + #{membership := promotable, target := Target} ?= VoterStatus, true ?= MI >= Target, E = {next_event, {command, {'$ra_join', #{ts => os:system_time(millisecond)}, - #{id => PeerID, - voter_status => VoterStatus#{non_voter => false}}, + #{id => PeerId, + voter_status => VoterStatus#{membership => voter}}, noreply}}}, [E | Effects] else @@ -2971,7 +2970,7 @@ required_quorum(Cluster) -> count_voters(Cluster) -> maps:fold( - fun (_, #{voter_status := #{non_voter := true}}, Count) -> + fun (_, #{voter_status := #{membership := promotable}}, Count) -> Count; (_, _, Count) -> Count + 1 diff --git a/src/ra_server_proc.erl b/src/ra_server_proc.erl index cda7699e..16326adf 100644 --- a/src/ra_server_proc.erl +++ b/src/ra_server_proc.erl @@ -7,6 +7,8 @@ %% @hidden -module(ra_server_proc). +-feature(maybe_expr, enable). + -behaviour(gen_statem). -compile({inline, [handle_raft_state/3]}). @@ -273,7 +275,7 @@ init(Config) -> do_init(#{id := Id, cluster_name := ClusterName} = Config0) -> Key = ra_lib:ra_server_id_to_local_name(Id), - true = ets:insert(ra_state, {Key, init, false}), %% can't vote while initializing + true = ets:insert(ra_state, {Key, init, unknown}), process_flag(trap_exit, true), Config = #{counter := Counter, system_config := SysConf} = maps:merge(config_defaults(Id), @@ -788,16 +790,16 @@ follower({call, From}, {log_fold, Fun, Term}, State) -> fold_log(From, Fun, Term, State); follower(EventType, Msg, #state{conf = #conf{name = Name}, server_state = SS0} = State0) -> - NonVoter0 = ra_server:get_non_voter(SS0), + Membership0 = ra_server:get_membership(SS0), case handle_follower(Msg, State0) of {follower, State1, Effects} -> {State2, Actions} = ?HANDLE_EFFECTS(Effects, EventType, State1), State = #state{server_state = SS} = follower_leader_change(State0, State2), - case ra_server:get_non_voter(SS) of - NonVoter0 -> + case ra_server:get_membership(SS) of + Membership0 -> ok; - NonVoter -> - true = ets:update_element(ra_state, Name, {3, NonVoter}) + Membership -> + true = ets:update_element(ra_state, Name, {3, Membership}) end, {keep_state, State, Actions}; {pre_vote, State1, Effects} -> @@ -1039,8 +1041,8 @@ format_status(Opt, [_PDict, StateName, handle_enter(RaftState, OldRaftState, #state{conf = #conf{name = Name}, server_state = ServerState0} = State) -> - NonVoter = ra_server:get_non_voter(ServerState0), - true = ets:insert(ra_state, {Name, RaftState, NonVoter}), + Membership = ra_server:get_membership(ServerState0), + true = ets:insert(ra_state, {Name, RaftState, Membership}), {ServerState, Effects} = ra_server:handle_state_enter(RaftState, ServerState0), case RaftState == leader orelse OldRaftState == leader of @@ -1522,13 +1524,17 @@ do_state_query(overview, State) -> do_state_query(machine, #{machine_state := MacState}) -> MacState; do_state_query(voters, #{cluster := Cluster}) -> - Voters = maps:filter(fun(_, Peer) -> - case maps:get(voter_status, Peer, undefined) of - #{non_voter := true} -> false; - _ -> true - end - end, Cluster), - maps:keys(Voters); + Vs = maps:fold(fun(K, V, Acc) -> + case maps:get(voter_status, V, undefined) of + undefined -> [K|Acc]; + S -> case maps:get(membership, S, undefined) of + undefined -> [K|Acc]; + voter -> [K|Acc]; + _ -> Acc + end + end + end, [], Cluster), + Vs; do_state_query(members, #{cluster := Cluster}) -> maps:keys(Cluster); do_state_query(initial_members, #{log := Log}) -> @@ -1738,9 +1744,9 @@ handle_tick_metrics(State) -> can_execute_locally(RaftState, TargetNode, #state{server_state = ServerState} = State) -> - NonVoter = ra_server:get_non_voter(ServerState), + Membership = ra_server:get_membership(ServerState), case RaftState of - follower when not NonVoter -> + follower when Membership == voter -> TargetNode == node(); leader when TargetNode =/= node() -> %% We need to evaluate whether to send the message. diff --git a/test/coordination_SUITE.erl b/test/coordination_SUITE.erl index 9d8202d3..e0c3116f 100644 --- a/test/coordination_SUITE.erl +++ b/test/coordination_SUITE.erl @@ -308,8 +308,8 @@ send_local_msg(Config) -> {ok, _, Leader} = ra:members(hd(NodeIds)), {ok, _, _} = ra:process_command(Leader, banana), New = #{id => NonVoter, - voter_status => #{non_voter => true, uid => <<"test">>, target => 999}, - non_voter => true, + voter_status => #{membership => promotable, uid => <<"test">>, target => 999}, + membership => promotable, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, NodeIds), @@ -347,8 +347,8 @@ local_log_effect(Config) -> {ok, _, Leader} = ra:members(hd(NodeIds)), {ok, _, _} = ra:process_command(Leader, banana), New = #{id => NonVoter, - voter_status => #{non_voter => true, uid => <<"test">>, target => 999}, - non_voter => true, + voter_status => #{membership => promotable, uid => <<"test">>, target => 999}, + membership => promotable, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, NodeIds), @@ -436,24 +436,24 @@ nonvoter_catches_up(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, non_voter => true, uid => <<"test">>}, + New = #{id => C, membership => promotable, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), - ?assertMatch(#{Group := #{non_voter := true}}, + ?assertMatch(#{Group := #{membership := promotable}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{non_voter := true}, + ?assertMatch(#{membership := promotable}, ra:key_metrics(C)), - ?assertMatch({ok, #{non_voter := true}, _}, + ?assertMatch({ok, #{membership := promotable}, _}, ra:member_overview(C)), await_condition( fun () -> - {ok, #{non_voter := NV}, _} = ra:member_overview(C), - not NV + {ok, #{membership := M}, _} = ra:member_overview(C), + M == voter end, 200), - ?assertMatch(#{Group := #{non_voter := false}}, + ?assertMatch(#{Group := #{membership := voter}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{non_voter := false}, + ?assertMatch(#{membership := voter}, ra:key_metrics(C)), [ok = slave:stop(S) || {_, S} <- ServerIds], @@ -471,26 +471,26 @@ nonvoter_catches_up_after_restart(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, non_voter => true, uid => <<"test">>}, + New = #{id => C, membership => promotable, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), - ?assertMatch(#{Group := #{non_voter := true}}, + ?assertMatch(#{Group := #{membership := promotable}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{non_voter := true}, + ?assertMatch(#{membership := promotable}, ra:key_metrics(C)), - ?assertMatch({ok, #{non_voter := true}, _}, + ?assertMatch({ok, #{membership := promotable}, _}, ra:member_overview(C)), ok = ra:stop_server(?SYS, C), ok = ra:restart_server(?SYS, C), await_condition( fun () -> - {ok, #{non_voter := NV}, _} = ra:member_overview(C), - not NV + {ok, #{membership := M}, _} = ra:member_overview(C), + M == voter end, 200), - ?assertMatch(#{Group := #{non_voter := false}}, + ?assertMatch(#{Group := #{membership := voter}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{non_voter := false}, + ?assertMatch(#{membership := voter}, ra:key_metrics(C)), [ok = slave:stop(S) || {_, S} <- ServerIds], @@ -508,26 +508,26 @@ nonvoter_catches_up_after_leader_restart(Config) -> || N <- lists:seq(1, 10000)], {ok, _, _} = ra:process_command(Leader, banana), - New = #{id => C, non_voter => true, uid => <<"test">>}, + New = #{id => C, membership => promotable, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, [A, B]), - ?assertMatch(#{Group := #{non_voter := true}}, + ?assertMatch(#{Group := #{membership := promotable}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{non_voter := true}, + ?assertMatch(#{membership := promotable}, ra:key_metrics(C)), - ?assertMatch({ok, #{non_voter := true}, _}, + ?assertMatch({ok, #{membership := promotable}, _}, ra:member_overview(C)), ok = ra:stop_server(?SYS, Leader), ok = ra:restart_server(?SYS, Leader), await_condition( fun () -> - {ok, #{non_voter := NV}, _} = ra:member_overview(C), - not NV + {ok, #{membership := M}, _} = ra:member_overview(C), + M == voter end, 200), - ?assertMatch(#{Group := #{non_voter := false}}, + ?assertMatch(#{Group := #{membership := voter}}, rpc:call(NodeC, ra_directory, overview, [?SYS])), - ?assertMatch(#{non_voter := false}, + ?assertMatch(#{membership := voter}, ra:key_metrics(C)), [ok = slave:stop(S) || {_, S} <- ServerIds], diff --git a/test/ra_2_SUITE.erl b/test/ra_2_SUITE.erl index f5ff9a1c..caccde6d 100644 --- a/test/ra_2_SUITE.erl +++ b/test/ra_2_SUITE.erl @@ -714,12 +714,12 @@ force_start_follower_as_single_member_nonvoter(Config) -> ServerId4 = ?config(server_id4, Config), UId4 = ?config(uid4, Config), Conf4 = conf(ClusterName, UId4, ServerId4, PrivDir, [ServerId3]), - {ok, _, _} = ra:add_member(ServerId3, #{id => ServerId4, non_voter => true, uid => <<"test">>}), + {ok, _, _} = ra:add_member(ServerId3, #{id => ServerId4, membership => promotable, uid => <<"test">>}), %% the membership has changed but member not running yet %% it is nonvoter and does not affect quorum size {ok, _, _} = ra:process_command(ServerId3, {enq, banana}), %% start new member - ok = ra:start_server(?SYS, Conf4#{non_voter=> true, uid => <<"test">>}), + ok = ra:start_server(?SYS, Conf4#{membership => promotable, uid => <<"test">>}), {ok, _, ServerId3} = ra:members(ServerId4), ok = enqueue(ServerId3, msg3), diff --git a/test/ra_SUITE.erl b/test/ra_SUITE.erl index 7177e0d3..895a27d3 100644 --- a/test/ra_SUITE.erl +++ b/test/ra_SUITE.erl @@ -1039,15 +1039,15 @@ new_nonvoter_knows_its_status(Config) -> % n2 had no time to catch up % in server state - {ok, #{uid := T, non_voter := true}, _} = ra:member_overview(N2), + {ok, #{uid := T, membership := promotable}, _} = ra:member_overview(N2), {ok, - #{cluster := #{N2 := #{voter_status := #{non_voter := true, + #{cluster := #{N2 := #{voter_status := #{membership := promotable, target := 3, uid := T}}}}, _} = ra:member_overview(N1), % in ets - #{servers := #{n1 := #{non_voter := false}, - n2 := #{non_voter := true}}} = ra:overview(?SYS), + #{servers := #{n1 := #{membership := voter}, + n2 := #{membership := promotable}}} = ra:overview(?SYS), ok. voter_gets_promoted_consistent_leader(Config) -> @@ -1076,7 +1076,7 @@ voter_gets_promoted_consistent_leader(Config) -> lists:map(fun(O) -> ?assertEqual(All, voters(O)) end, overviews(N1)), % in ets #{servers := Servers} = ra:overview(?SYS), - lists:map(fun({Name, _}) -> #{Name := #{non_voter := false}} = Servers end, All), + lists:map(fun({Name, _}) -> #{Name := #{membership := voter}} = Servers end, All), ok. voter_gets_promoted_new_leader(Config) -> @@ -1101,7 +1101,7 @@ voter_gets_promoted_new_leader(Config) -> lists:map(fun(O) -> ?assertEqual(All, voters(O)) end, overviews(N1)), % in ets #{servers := Servers} = ra:overview(?SYS), - lists:map(fun({Name, _}) -> #{Name := #{non_voter := false}} = Servers end, All), + lists:map(fun({Name, _}) -> #{Name := #{membership := voter}} = Servers end, All), ok. get_gen_statem_status(Ref) -> @@ -1178,7 +1178,7 @@ start_and_join({ClusterName, _} = ServerRef, {_, _} = New) -> start_and_join_nonvoter({ClusterName, _} = ServerRef, {_, _} = New) -> UId = ra:new_uid(ra_lib:to_binary(ClusterName)), - Server = #{id => New, non_voter => true, uid => UId}, + Server = #{id => New, membership => promotable, uid => UId}, {ok, _, _} = ra:add_member(ServerRef, Server), ok = ra:start_server(default, ClusterName, Server, add_machine(), [ServerRef]), ok. @@ -1218,7 +1218,7 @@ overviews(Node) -> [ra:member_overview(P) || {_, _} = P <- Members]. voters({ok, #{cluster := Peers}, _} = _Overview) -> - [Id || {Id, Status} <- maps:to_list(Peers), not maps:get(non_voter, Status, false)]. + [Id || {Id, Status} <- maps:to_list(Peers), maps:get(membership, Status, voter) == voter]. %% machine impl init(_) -> 0. diff --git a/test/ra_server_SUITE.erl b/test/ra_server_SUITE.erl index c435c221..8675f325 100644 --- a/test/ra_server_SUITE.erl +++ b/test/ra_server_SUITE.erl @@ -280,7 +280,7 @@ election_timeout(_Config) -> ra_server:handle_follower(Msg, State), % non-voters ignore election_timeout - NVState = State#{voter_status => #{non_voter => true}}, + NVState = State#{voter_status => #{membership => promotable}}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), % pre_vote @@ -806,7 +806,7 @@ candidate_handles_append_entries_rpc(_Config) -> append_entries_reply_success_promotes_nonvoter(_Config) -> N1 = ?N1, N2 = ?N2, N3 = ?N3, - NonVoter = #{non_voter => true, target => 3, uid => <<"uid">>}, + NonVoter = #{membership => promotable, target => 3, uid => <<"uid">>}, Cluster = #{N1 => new_peer_with(#{next_index => 5, match_index => 4}), N2 => new_peer_with(#{next_index => 1, match_index => 0, commit_index_sent => 3, @@ -830,7 +830,7 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> [{next_event, info, pipeline_rpcs}, {next_event, {command, {'$ra_join', _, #{id := N2, - voter_status := #{non_voter := false, uid := <<"uid">>}}, + voter_status := #{membership := voter, uid := <<"uid">>}}, noreply}} = RaJoin} ]} = ra_server:handle_leader({N2, Ack}, State0), @@ -852,7 +852,7 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> % ra_join translates into cluster update {leader, #{cluster := #{N2 := #{next_index := 5, match_index := 3, - voter_status := #{non_voter := false}}}, + voter_status := #{membership := voter}}}, cluster_change_permitted := false, commit_index := 1, last_applied := 1, @@ -863,7 +863,7 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> prev_log_term = 5, leader_commit = 1, entries = [{4, 5, {'$ra_cluster_change', _, - #{N2 := #{voter_status := #{non_voter := false}}}, + #{N2 := #{voter_status := #{membership := voter}}}, _}}]}}, {send_rpc, N2, #append_entries_rpc{term = 5, leader_id = N1, @@ -871,7 +871,7 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> prev_log_term = 5, leader_commit = 1, entries = [{4, 5, {'$ra_cluster_change', _, - #{N2 := #{voter_status := #{non_voter := false}}}, + #{N2 := #{voter_status := #{membership := voter}}}, _}}]}} ]} = ra_server:handle_leader(RaJoin, State2), @@ -1013,7 +1013,7 @@ follower_request_vote(_Config) -> State), % non-voters ignore request_vote_rpc - NVState = State#{voter_status => #{non_voter => true}}, + NVState = State#{voter_status => #{membership => promotable}}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), ok. @@ -1132,7 +1132,7 @@ follower_pre_vote(_Config) -> State), % non-voters ignore pre_vote_rpc - NVState = State#{voter_status => #{non_voter => true}}, + NVState = State#{voter_status => #{membership => promotable}}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), ok. @@ -1434,7 +1434,7 @@ leader_server_join(_Config) -> prev_log_term = 5, leader_commit = 3}} | _] = Effects, - undefined = maps:get(non_voter, N4Peer, undefined), + undefined = maps:get(membership, N4Peer, undefined), ok. leader_server_join_nonvoter(_Config) -> @@ -1448,9 +1448,9 @@ leader_server_join_nonvoter(_Config) -> {leader, #{cluster := #{N1 := _, N2 := _, N3 := _, N4 := _}, cluster_change_permitted := false} = _State1, Effects} = ra_server:handle_leader({command, {'$ra_join', meta(), - #{id => N4, non_voter => true, uid => <<"uid">>}, await_consensus}}, State0), + #{id => N4, membership => promotable, uid => <<"uid">>}, await_consensus}}, State0), % new member should join as non-voter - Status = #{non_voter => true, uid => <<"uid">>, target => ra_log:next_index(Log)}, + Status = #{membership => promotable, uid => <<"uid">>, target => ra_log:next_index(Log)}, [ {send_rpc, N4, #append_entries_rpc{entries = @@ -1611,12 +1611,12 @@ leader_applies_new_cluster_nonvoter(_Config) -> N3 => new_peer_with(#{next_index => 4, match_index => 3})}, State = (base_state(3, ?FUNCTION_NAME))#{cluster => OldCluster}, - Command = {command, {'$ra_join', meta(), #{id => N4, non_voter => true, uid => <<"uid">>}, await_consensus}}, + Command = {command, {'$ra_join', meta(), #{id => N4, membership => promotable, uid => <<"uid">>}, await_consensus}}, % cluster records index and term it was applied to determine whether it has % been applied {leader, #{cluster_index_term := {4, 5}, cluster := #{N1 := _, N2 := _, - N3 := _, N4 := #{voter_status := #{non_voter := true, + N3 := _, N4 := #{voter_status := #{membership := promotable, uid := <<"uid">>}}}} = State1, _} = ra_server:handle_leader(Command, State), {leader, State2, _} = @@ -2129,23 +2129,23 @@ leader_received_append_entries_reply_and_promotes_voter(_config) -> last_index = 4, last_term = 5}, % Permanent voter - State1 = set_peer_voter_status(State, N3, #{non_voter => false}), + State1 = set_peer_voter_status(State, N3, #{membership => false}), {leader, _, [{next_event,info,pipeline_rpcs}] } = ra_server:handle_leader({N3, AER}, State1), % Permanent non-voter - State2 = set_peer_voter_status(State, N3, #{non_voter => true}), + State2 = set_peer_voter_status(State, N3, #{membership => promotable}), {leader, _, [{next_event,info,pipeline_rpcs}] } = ra_server:handle_leader({N3, AER}, State2), % Promotion State3 = set_peer_voter_status(State, N3, - #{non_voter => true, target => 4}), + #{membership => promotable, target => 4}), {leader, _, [{next_event,info,pipeline_rpcs}, - {next_event, {command, {'$ra_join', _, #{id := N3, voter_status := #{non_voter := false}}, _}}}] + {next_event, {command, {'$ra_join', _, #{id := N3, voter_status := #{membership := voter}}, _}}}] } = ra_server:handle_leader({N3, AER}, State3). follower_heartbeat(_Config) -> From 4ac1d502175f4a2b575bb6193ad83bdc5cddfc7b Mon Sep 17 00:00:00 2001 From: Alex Valiushko Date: Wed, 20 Sep 2023 21:22:35 -0700 Subject: [PATCH 6/8] fix types --- src/ra.hrl | 2 +- src/ra_server.erl | 28 ++++++++++---------- test/coordination_SUITE.erl | 6 ++--- test/ra_server_SUITE.erl | 51 +++++++++++++++++++++---------------- 4 files changed, 47 insertions(+), 40 deletions(-) diff --git a/src/ra.hrl b/src/ra.hrl index e48f10a8..269204c8 100644 --- a/src/ra.hrl +++ b/src/ra.hrl @@ -53,7 +53,7 @@ suspended | disconnected. --type ra_membership() :: voter | promotable | unknown. +-type ra_membership() :: voter | promotable | non_voter | unknown. -type ra_voter_status() :: #{membership => ra_membership(), uid => ra_uid(), diff --git a/src/ra_server.erl b/src/ra_server.erl index 967fad4a..a16fe40a 100644 --- a/src/ra_server.erl +++ b/src/ra_server.erl @@ -200,7 +200,9 @@ ra_event_formatter => {module(), atom(), [term()]}, counter => counters:counters_ref(), membership => ra_membership(), - system_config => ra_system:config()}. + system_config => ra_system:config(), + has_changed => boolean() + }. -type mutable_config() :: #{cluster_name => ra_cluster_name(), metrics_key => term(), @@ -1121,17 +1123,17 @@ handle_follower({ra_log_event, Evt}, State = #{log := Log0}) -> {follower, State#{log => Log}, Effects}; handle_follower(#pre_vote_rpc{}, #{cfg := #cfg{log_id = LogId}, - voter_status := #{membership := promotable} = VoterStatus} = State) -> + membership := Membership} = State) when Membership =/= voter -> ?DEBUG("~s: follower ignored pre_vote_rpc, non-voter: ~p0", - [LogId, VoterStatus]), + [LogId, Membership]), {follower, State, []}; handle_follower(#pre_vote_rpc{} = PreVote, State) -> process_pre_vote(follower, PreVote, State); handle_follower(#request_vote_rpc{}, #{cfg := #cfg{log_id = LogId}, - voter_status := #{membership := promotable} = VoterStatus} = State) -> + membership := Membership} = State) when Membership =/= voter -> ?DEBUG("~s: follower ignored request_vote_rpc, non-voter: ~p0", - [LogId, VoterStatus]), + [LogId, Membership]), {follower, State, []}; handle_follower(#request_vote_rpc{candidate_id = Cand, term = Term}, #{current_term := Term, voted_for := VotedFor, @@ -1231,9 +1233,9 @@ handle_follower(#append_entries_reply{}, State) -> {follower, State, []}; handle_follower(election_timeout, #{cfg := #cfg{log_id = LogId}, - voter_status := #{membership := promotable} = VoterStatus} = State) -> + membership := Membership} = State) when Membership =/= voter -> ?DEBUG("~s: follower ignored election_timeout, non-voter: ~p0", - [LogId, VoterStatus]), + [LogId, Membership]), {follower, State, []}; handle_follower(election_timeout, State) -> call_for_election(pre_vote, State); @@ -2900,7 +2902,7 @@ already_member(State) -> %%% Voter status helpers %%% ==================== --spec ensure_promotion_target(ra_voter_status(), ra_index()) -> +-spec ensure_promotion_target(ra_voter_status(), ra_server_state()) -> {ok, ra_voter_status()} | {error, term()}. ensure_promotion_target(#{membership := promotable, target := _, uid := _} = Status, _) -> @@ -2916,8 +2918,8 @@ ensure_promotion_target(Voter, _) -> %% Get membership of a given Id+UId from a (possibly new) cluster. -spec get_membership(ra_cluster() | ra_cluster_snapshot() | ra_cluster_servers(), - ra_server_id(), ra_uid(), boolean()) -> - boolean(). + ra_server_id(), ra_uid(), ra_membership()) -> + ra_membership(). get_membership(_Cluster, _PeerId, _UId, Default) when is_list(_Cluster) -> %% Legacy cluster snapshot does not retain voter_status. Default; @@ -2932,15 +2934,15 @@ get_membership(Cluster, PeerId, UId, Default) -> %% Get this node's membership from a (possibly new) cluster. %% Defaults to last known-locally value. -spec get_membership(ra_cluster() | ra_cluster_snapshot() | ra_cluster_servers(), - ra_state()) -> - boolean(). + ra_server_state()) -> + ra_membership(). get_membership(Cluster, #{cfg := #cfg{id = Id, uid = UId}} = State) -> Default = maps:get(membership, State, voter), get_membership(Cluster, Id, UId, Default). %% Get this node's membership. %% Defaults to last known-locally value. --spec get_membership(ra_state()) -> boolean(). +-spec get_membership(ra_server_state()) -> ra_membership(). get_membership(#{cfg := #cfg{id = Id, uid = UId}, cluster := Cluster} = State) -> Default = maps:get(membership, State, voter), get_membership(Cluster, Id, UId, Default). diff --git a/test/coordination_SUITE.erl b/test/coordination_SUITE.erl index e0c3116f..4e8f0653 100644 --- a/test/coordination_SUITE.erl +++ b/test/coordination_SUITE.erl @@ -308,8 +308,7 @@ send_local_msg(Config) -> {ok, _, Leader} = ra:members(hd(NodeIds)), {ok, _, _} = ra:process_command(Leader, banana), New = #{id => NonVoter, - voter_status => #{membership => promotable, uid => <<"test">>, target => 999}, - membership => promotable, + membership => non_voter, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, NodeIds), @@ -347,8 +346,7 @@ local_log_effect(Config) -> {ok, _, Leader} = ra:members(hd(NodeIds)), {ok, _, _} = ra:process_command(Leader, banana), New = #{id => NonVoter, - voter_status => #{membership => promotable, uid => <<"test">>, target => 999}, - membership => promotable, + membership => non_voter, uid => <<"test">>}, {ok, _, _} = ra:add_member(A, New), ok = ra:start_server(?SYS, ClusterName, New, Machine, NodeIds), diff --git a/test/ra_server_SUITE.erl b/test/ra_server_SUITE.erl index 8675f325..e431903a 100644 --- a/test/ra_server_SUITE.erl +++ b/test/ra_server_SUITE.erl @@ -207,7 +207,7 @@ init_test(_Config) -> voted_for := some_server} = ra_server_init(InitConf), % snapshot SnapshotMeta = #{index => 3, term => 5, - cluster => maps:keys(Cluster), + cluster => dehydrate_cluster(Cluster), machine_version => 1}, SnapshotData = "hi1+2+3", {LogS, _, _} = ra_log_memory:install_snapshot(SnapshotMeta, SnapshotData, Log0), @@ -217,7 +217,7 @@ init_test(_Config) -> machine_state := "hi1+2+3", cluster := ClusterOut, voted_for := some_server} = ra_server_init(InitConf), - ?assertEqual(maps:keys(Cluster), maps:keys(ClusterOut)), + ?assertEqual(dehydrate_cluster(Cluster), dehydrate_cluster(ClusterOut)), ok. recover_restores_cluster_changes(_Config) -> @@ -280,7 +280,7 @@ election_timeout(_Config) -> ra_server:handle_follower(Msg, State), % non-voters ignore election_timeout - NVState = State#{voter_status => #{membership => promotable}}, + NVState = State#{membership => promotable}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), % pre_vote @@ -580,7 +580,7 @@ follower_aer_term_mismatch_snapshot(_Config) -> Log0 = maps:get(log, State0), Meta = #{index => 3, term => 5, - cluster => [], + cluster => #{}, machine_version => 1}, Data = <<"hi3">>, {Log, _, _} = ra_log_memory:install_snapshot(Meta, Data, Log0), @@ -731,7 +731,7 @@ follower_catchup_condition(_Config) -> chunk_state = {1, last}, meta = #{index => 99, term => 99, - cluster => [], + cluster => #{}, machine_version => 0}, data = []}, {follower, State, [_NextEvent]} = @@ -830,7 +830,8 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> [{next_event, info, pipeline_rpcs}, {next_event, {command, {'$ra_join', _, #{id := N2, - voter_status := #{membership := voter, uid := <<"uid">>}}, + voter_status := #{membership := voter, + uid := <<"uid">>}}, noreply}} = RaJoin} ]} = ra_server:handle_leader({N2, Ack}, State0), @@ -852,7 +853,8 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> % ra_join translates into cluster update {leader, #{cluster := #{N2 := #{next_index := 5, match_index := 3, - voter_status := #{membership := voter}}}, + voter_status := #{membership := voter, + uid := <<"uid">>}}}, cluster_change_permitted := false, commit_index := 1, last_applied := 1, @@ -863,7 +865,8 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> prev_log_term = 5, leader_commit = 1, entries = [{4, 5, {'$ra_cluster_change', _, - #{N2 := #{voter_status := #{membership := voter}}}, + #{N2 := #{voter_status := #{membership := voter, + uid := <<"uid">>}}}, _}}]}}, {send_rpc, N2, #append_entries_rpc{term = 5, leader_id = N1, @@ -871,7 +874,8 @@ append_entries_reply_success_promotes_nonvoter(_Config) -> prev_log_term = 5, leader_commit = 1, entries = [{4, 5, {'$ra_cluster_change', _, - #{N2 := #{voter_status := #{membership := voter}}}, + #{N2 := #{voter_status := #{membership := voter, + uid := <<"uid">>}}}, _}}]}} ]} = ra_server:handle_leader(RaJoin, State2), @@ -1013,7 +1017,7 @@ follower_request_vote(_Config) -> State), % non-voters ignore request_vote_rpc - NVState = State#{voter_status => #{membership => promotable}}, + NVState = State#{membership => promotable}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), ok. @@ -1132,7 +1136,7 @@ follower_pre_vote(_Config) -> State), % non-voters ignore pre_vote_rpc - NVState = State#{voter_status => #{membership => promotable}}, + NVState = State#{membership => promotable}, {follower, NVState, []} = ra_server:handle_follower(Msg, NVState), ok. @@ -1372,7 +1376,7 @@ follower_install_snapshot_machine_version(_Config) -> %% by install snapshot rpc SnapMeta = #{index => 4, term => 5, - cluster => [?N1, ?N2, ?N3], + cluster => #{?N1 => #{}, ?N2 => #{}, ?N3 => #{}}, machine_version => MacVer}, SnapData = <<"hi4_v2">>, @@ -1823,7 +1827,7 @@ pre_vote_election_reverts(_Config) -> ISR = #install_snapshot_rpc{term = 5, leader_id = N2, meta = #{index => 3, term => 5, - cluster => [], + cluster => #{}, machine_version => 0}, chunk_state = {1, last}, data = []}, @@ -1860,7 +1864,7 @@ leader_receives_install_snapshot_rpc(_Config) -> ISRpc = #install_snapshot_rpc{term = Term + 1, leader_id = ?N5, meta = #{index => Idx, term => Term, - cluster => [], + cluster => #{}, machine_version => 0}, chunk_state = {1, last}, data = []}, @@ -1891,8 +1895,7 @@ follower_installs_snapshot(_Config) -> fun (_) -> {#{index => Idx, term => Term, - cluster => maps:keys(Config), - cluster_state => Config, + cluster => dehydrate_cluster(Config), machine_version => 0}, []} end), @@ -1919,7 +1922,7 @@ follower_ignores_installs_snapshot_with_higher_machine_version(_Config) -> ISRpc = #install_snapshot_rpc{term = Term, leader_id = N1, meta = #{index => Idx, term => LastTerm, - cluster => maps:keys(Config), + cluster => dehydrate_cluster(Config), machine_version => 1}, chunk_state = {1, last}, data = []}, @@ -1981,7 +1984,7 @@ snapshotted_follower_received_append_entries(_Config) -> fun (_) -> {#{index => Idx, term => Term, - cluster => maps:keys(Config), + cluster => dehydrate_cluster(Config), machine_version => 0}, []} end), @@ -2129,13 +2132,13 @@ leader_received_append_entries_reply_and_promotes_voter(_config) -> last_index = 4, last_term = 5}, % Permanent voter - State1 = set_peer_voter_status(State, N3, #{membership => false}), + State1 = set_peer_voter_status(State, N3, #{membership => voter}), {leader, _, [{next_event,info,pipeline_rpcs}] } = ra_server:handle_leader({N3, AER}, State1), % Permanent non-voter - State2 = set_peer_voter_status(State, N3, #{membership => promotable}), + State2 = set_peer_voter_status(State, N3, #{membership => non_voter}), {leader, _, [{next_event,info,pipeline_rpcs}] } = ra_server:handle_leader({N3, AER}, State2), @@ -2808,7 +2811,11 @@ snap_meta(Idx, Term) -> snap_meta(Idx, Term, Cluster) -> #{index => Idx, term => Term, - cluster => maps:keys(Cluster), - cluster_state => Cluster, + cluster => dehydrate_cluster(Cluster), machine_version => 0}. +dehydrate_cluster(Cluster) -> + maps:map(fun(_, V) -> + maps:with([voter_status], V) + end, Cluster). + From e0f292f5cdf8c96671a1c08d48d2ac5f2167b456 Mon Sep 17 00:00:00 2001 From: Alex Valiushko Date: Thu, 21 Sep 2023 12:59:11 -0700 Subject: [PATCH 7/8] typo --- src/ra.hrl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ra.hrl b/src/ra.hrl index 269204c8..8f1538cf 100644 --- a/src/ra.hrl +++ b/src/ra.hrl @@ -43,7 +43,7 @@ %% Subset of ra_server:ra_server_config(). %% Both `ra:add_member` and `ra:start_server` must be called with the same values. -type ra_new_server() :: #{id := ra_server_id(), - % Defaults to `voter` is absent. + % Defaults to `voter` if absent. membership => ra_membership(), % Required for `promotable` in the above. uid => ra_uid()}. From 80ccd9142d7ab9b3e1fc9759df31b09147899c27 Mon Sep 17 00:00:00 2001 From: Alex Valiushko Date: Fri, 22 Sep 2023 10:34:02 -0700 Subject: [PATCH 8/8] remove maybe_expr --- src/ra_server.erl | 33 +++++++++++++++++---------------- src/ra_server_proc.erl | 2 -- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/ra_server.erl b/src/ra_server.erl index a16fe40a..405b44d9 100644 --- a/src/ra_server.erl +++ b/src/ra_server.erl @@ -6,8 +6,6 @@ %% -module(ra_server). --feature(maybe_expr, enable). - -include("ra.hrl"). -include("ra_server.hrl"). @@ -2947,22 +2945,25 @@ get_membership(#{cfg := #cfg{id = Id, uid = UId}, cluster := Cluster} = State) - Default = maps:get(membership, State, voter), get_membership(Cluster, Id, UId, Default). --spec maybe_promote_peer(ra_server_id(), ra_server_state(), effects()) -> effects(). +-spec maybe_promote_peer(ra_server_id(), ra_server_state(), effects()) -> + effects(). maybe_promote_peer(PeerId, #{cluster := Cluster}, Effects) -> - maybe + case Cluster of #{PeerId := #{match_index := MI, - voter_status := VoterStatus}} ?= Cluster, - #{membership := promotable, target := Target} ?= VoterStatus, - true ?= MI >= Target, - E = {next_event, - {command, {'$ra_join', - #{ts => os:system_time(millisecond)}, - #{id => PeerId, - voter_status => VoterStatus#{membership => voter}}, - noreply}}}, - [E | Effects] - else - _ -> Effects + voter_status := #{membership := promotable, + target := Target} = OldStatus}} when + MI >= Target -> + Promote = {next_event, + {command, {'$ra_join', + #{ts => os:system_time(millisecond)}, + #{id => PeerId, + voter_status => OldStatus#{ + membership => voter + }}, + noreply}}}, + [Promote | Effects]; + _ -> + Effects end. -spec required_quorum(ra_cluster()) -> pos_integer(). diff --git a/src/ra_server_proc.erl b/src/ra_server_proc.erl index 16326adf..c985aa4a 100644 --- a/src/ra_server_proc.erl +++ b/src/ra_server_proc.erl @@ -7,8 +7,6 @@ %% @hidden -module(ra_server_proc). --feature(maybe_expr, enable). - -behaviour(gen_statem). -compile({inline, [handle_raft_state/3]}).