Merge pull request #509 from rabbitmq/fix-assertion

Fix off by one in follower assertion.
rabbitmq · Feb 6, 2025 · 4a0f8f4 · 4a0f8f4
2 parents 2022997 + ac44c4d
commit 4a0f8f4
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 3 deletions.
diff --git a/src/ra_server.erl b/src/ra_server.erl
@@ -537,6 +537,7 @@ handle_leader({PeerId, #append_entries_reply{success = false,
                           % entry was not found - simply set next index to
                           ?DEBUG("~ts: setting next index for ~w ~b",
                                  [LogId, PeerId, NextIdx]),
+                          %% TODO: match index should not be set here surely??
                           {Peer0#{match_index => LastIdx,
                                   next_index => NextIdx}, L};
                       % entry exists we can forward
@@ -1189,9 +1190,10 @@ handle_follower(#append_entries_rpc{term = Term,
                                [] when LastIdx > PLIdx ->
                                    %% if no entries were sent we need to reset
                                    %% last index to match the leader
-                                   ?DEBUG("~ts: resetting last index to ~b from ~b in term ~b",
+                                   ?DEBUG("~ts: resetting last index to ~b from ~b "
+                                          "in term ~b",
                                          [LogId, PLIdx, LastIdx, Term]),
-                                   ?assertNot(PLIdx =< CommitIndex),
+                                   ?assertNot(PLIdx < CommitIndex),
                                    {ok, L} = ra_log:set_last_index(PLIdx, Log1),
                                    L;
                                _ ->
@@ -3449,7 +3451,8 @@ required_quorum(Cluster) ->
 
 count_voters(Cluster) ->
     maps:fold(
-      fun (_, #{voter_status := #{membership := Membership}}, Count) when Membership =/= voter ->
+      fun (_, #{voter_status := #{membership := Membership}}, Count)
+            when Membership =/= voter ->
               Count;
           (_, _, Count) ->
               Count + 1

diff --git a/test/ra_server_SUITE.erl b/test/ra_server_SUITE.erl
@@ -68,6 +68,8 @@ all() ->
      follower_aer_3,
      follower_aer_4,
      follower_aer_5,
+     follower_aer_6,
+     follower_aer_7,
      follower_catchup_condition,
      wal_down_condition_follower,
      wal_down_condition_leader,
@@ -557,6 +559,82 @@ follower_aer_5(_Config) ->
                                        last_index = 3}, M),
     ok.
 
+follower_aer_6(_Config) ->
+    N1 = ?N1, N5 = ?N5,
+    %% Scenario
+    %% Leader with smaller log is elected and sends empty aer
+    %% Follower should truncate it's log and reply with an appropriate
+    %% next index
+    Init = empty_state(3, n2),
+    AER1 = #append_entries_rpc{term = 1, leader_id = N1,
+                               prev_log_index = 0,
+                               prev_log_term = 0,
+                               leader_commit = 3,
+                               entries = [
+                                          entry(1, 1, one),
+                                          entry(2, 1, two),
+                                          entry(3, 1, tre),
+                                          entry(4, 1, for)
+                                         ]},
+    %% set up follower state
+    {follower, State00, _} = ra_server:handle_follower(AER1, Init),
+    %% TODO also test when written even occurs after
+    {follower, #{last_applied := 3} = State0, _} =
+        ra_server:handle_follower(written_evt(1, {4, 4}), State00),
+    % now an AER from another leader in a higher term is received
+    % This is what the leader sends immediately before committing it;s noop
+    AER2 = #append_entries_rpc{term = 2, leader_id = N5,
+                               prev_log_index = 3,
+                               prev_log_term = 1,
+                               leader_commit = 3,
+                               entries = []},
+    {follower, _State1, Effects} = ra_server:handle_follower(AER2, State0),
+    {cast, N5, {_, M}} = hd(Effects),
+    ?assertMatch(#append_entries_reply{next_index = 4,
+                                       last_term = 1,
+                                       last_index = 3}, M),
+    ok.
+
+follower_aer_7(_Config) ->
+    N1 = ?N1, N5 = ?N5,
+    %% Scenario
+    %% Leader with smaller log is elected and sends empty aer
+    %% Follower should truncate it's log and reply with an appropriate
+    %% next index
+    Init = empty_state(3, n2),
+    AER1 = #append_entries_rpc{term = 1, leader_id = N1,
+                               prev_log_index = 0,
+                               prev_log_term = 0,
+                               leader_commit = 3,
+                               entries = [
+                                          entry(1, 1, one),
+                                          entry(2, 1, two),
+                                          entry(3, 1, tre),
+                                          entry(4, 1, for)
+                                         ]},
+    %% set up follower state
+    {follower, State00, _} = ra_server:handle_follower(AER1, Init),
+    %% TODO also test when written even occurs after
+    {follower, #{last_applied := 3} = State0, _} =
+        ra_server:handle_follower(written_evt(1, {4, 4}), State00),
+    % now an AER from another leader in a higher term is received
+    % This is what the leader sends immediately before committing it;s noop
+    AER2 = #append_entries_rpc{term = 2,
+                               leader_id = N5,
+                               prev_log_index = 3,
+                               prev_log_term = 1,
+                               leader_commit = 4,
+                               entries = [
+                                          entry(4, 2, for2)
+                                         ]},
+    {follower, State1, _} = ra_server:handle_follower(AER2, State0),
+    {follower, #{last_applied := 4} = _State, Effects} =
+        ra_server:handle_follower(written_evt(2, {4, 4}), State1),
+    {cast, N5, {_, M}} = hd(Effects),
+    ?assertMatch(#append_entries_reply{next_index = 5,
+                                       last_term = 2,
+                                       last_index = 4}, M),
+    ok.
 
 follower_aer_term_mismatch(_Config) ->
     State = (base_state(3, ?FUNCTION_NAME))#{last_applied => 2,