Skip to content

Commit 4a0f8f4

Browse files
authored
Merge pull request #509 from rabbitmq/fix-assertion
Fix off by one in follower assertion.
2 parents 2022997 + ac44c4d commit 4a0f8f4

File tree

2 files changed

+84
-3
lines changed

2 files changed

+84
-3
lines changed

src/ra_server.erl

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,7 @@ handle_leader({PeerId, #append_entries_reply{success = false,
537537
% entry was not found - simply set next index to
538538
?DEBUG("~ts: setting next index for ~w ~b",
539539
[LogId, PeerId, NextIdx]),
540+
%% TODO: match index should not be set here surely??
540541
{Peer0#{match_index => LastIdx,
541542
next_index => NextIdx}, L};
542543
% entry exists we can forward
@@ -1189,9 +1190,10 @@ handle_follower(#append_entries_rpc{term = Term,
11891190
[] when LastIdx > PLIdx ->
11901191
%% if no entries were sent we need to reset
11911192
%% last index to match the leader
1192-
?DEBUG("~ts: resetting last index to ~b from ~b in term ~b",
1193+
?DEBUG("~ts: resetting last index to ~b from ~b "
1194+
"in term ~b",
11931195
[LogId, PLIdx, LastIdx, Term]),
1194-
?assertNot(PLIdx =< CommitIndex),
1196+
?assertNot(PLIdx < CommitIndex),
11951197
{ok, L} = ra_log:set_last_index(PLIdx, Log1),
11961198
L;
11971199
_ ->
@@ -3449,7 +3451,8 @@ required_quorum(Cluster) ->
34493451

34503452
count_voters(Cluster) ->
34513453
maps:fold(
3452-
fun (_, #{voter_status := #{membership := Membership}}, Count) when Membership =/= voter ->
3454+
fun (_, #{voter_status := #{membership := Membership}}, Count)
3455+
when Membership =/= voter ->
34533456
Count;
34543457
(_, _, Count) ->
34553458
Count + 1

test/ra_server_SUITE.erl

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ all() ->
6868
follower_aer_3,
6969
follower_aer_4,
7070
follower_aer_5,
71+
follower_aer_6,
72+
follower_aer_7,
7173
follower_catchup_condition,
7274
wal_down_condition_follower,
7375
wal_down_condition_leader,
@@ -557,6 +559,82 @@ follower_aer_5(_Config) ->
557559
last_index = 3}, M),
558560
ok.
559561

562+
follower_aer_6(_Config) ->
563+
N1 = ?N1, N5 = ?N5,
564+
%% Scenario
565+
%% Leader with smaller log is elected and sends empty aer
566+
%% Follower should truncate it's log and reply with an appropriate
567+
%% next index
568+
Init = empty_state(3, n2),
569+
AER1 = #append_entries_rpc{term = 1, leader_id = N1,
570+
prev_log_index = 0,
571+
prev_log_term = 0,
572+
leader_commit = 3,
573+
entries = [
574+
entry(1, 1, one),
575+
entry(2, 1, two),
576+
entry(3, 1, tre),
577+
entry(4, 1, for)
578+
]},
579+
%% set up follower state
580+
{follower, State00, _} = ra_server:handle_follower(AER1, Init),
581+
%% TODO also test when written even occurs after
582+
{follower, #{last_applied := 3} = State0, _} =
583+
ra_server:handle_follower(written_evt(1, {4, 4}), State00),
584+
% now an AER from another leader in a higher term is received
585+
% This is what the leader sends immediately before committing it;s noop
586+
AER2 = #append_entries_rpc{term = 2, leader_id = N5,
587+
prev_log_index = 3,
588+
prev_log_term = 1,
589+
leader_commit = 3,
590+
entries = []},
591+
{follower, _State1, Effects} = ra_server:handle_follower(AER2, State0),
592+
{cast, N5, {_, M}} = hd(Effects),
593+
?assertMatch(#append_entries_reply{next_index = 4,
594+
last_term = 1,
595+
last_index = 3}, M),
596+
ok.
597+
598+
follower_aer_7(_Config) ->
599+
N1 = ?N1, N5 = ?N5,
600+
%% Scenario
601+
%% Leader with smaller log is elected and sends empty aer
602+
%% Follower should truncate it's log and reply with an appropriate
603+
%% next index
604+
Init = empty_state(3, n2),
605+
AER1 = #append_entries_rpc{term = 1, leader_id = N1,
606+
prev_log_index = 0,
607+
prev_log_term = 0,
608+
leader_commit = 3,
609+
entries = [
610+
entry(1, 1, one),
611+
entry(2, 1, two),
612+
entry(3, 1, tre),
613+
entry(4, 1, for)
614+
]},
615+
%% set up follower state
616+
{follower, State00, _} = ra_server:handle_follower(AER1, Init),
617+
%% TODO also test when written even occurs after
618+
{follower, #{last_applied := 3} = State0, _} =
619+
ra_server:handle_follower(written_evt(1, {4, 4}), State00),
620+
% now an AER from another leader in a higher term is received
621+
% This is what the leader sends immediately before committing it;s noop
622+
AER2 = #append_entries_rpc{term = 2,
623+
leader_id = N5,
624+
prev_log_index = 3,
625+
prev_log_term = 1,
626+
leader_commit = 4,
627+
entries = [
628+
entry(4, 2, for2)
629+
]},
630+
{follower, State1, _} = ra_server:handle_follower(AER2, State0),
631+
{follower, #{last_applied := 4} = _State, Effects} =
632+
ra_server:handle_follower(written_evt(2, {4, 4}), State1),
633+
{cast, N5, {_, M}} = hd(Effects),
634+
?assertMatch(#append_entries_reply{next_index = 5,
635+
last_term = 2,
636+
last_index = 4}, M),
637+
ok.
560638

561639
follower_aer_term_mismatch(_Config) ->
562640
State = (base_state(3, ?FUNCTION_NAME))#{last_applied => 2,

0 commit comments

Comments
 (0)