Unverified Commit 600cf4fe authored by Shaya Potter's avatar Shaya Potter Committed by GitHub
Browse files

only promote node to voting when its nonvoting has been applied (#12)

before we were able to promote a "ready" but not applied nonvoting node to a voting state, which means it would change the calculation as could vote for itself to be added.

* fix tests that check for has_sufficient_logs

because of a fix put in to not allow a node to be added until its "addition" as a non voter was comitted, that broke existing tests where that flag wasn't being set and testing that the nodes could be promoted

* add a test that has_sufficient_logs will fail if addition not committed yet

code before would add a node as a voter (via has_sufficient_logs) even if the initial non voter addition wasn't committed

the test for this in the code actually tested after setting addition_committed.  add a test that does the same without that flag being set and expects that has_sufficient_logs will not be set
parent ad19243c
...@@ -362,6 +362,7 @@ int raft_recv_appendentries_response(raft_server_t* me_, ...@@ -362,6 +362,7 @@ int raft_recv_appendentries_response(raft_server_t* me_,
!raft_voting_change_is_in_progress(me_) && !raft_voting_change_is_in_progress(me_) &&
raft_get_current_idx(me_) <= r->current_idx + 1 && raft_get_current_idx(me_) <= r->current_idx + 1 &&
!raft_node_is_voting_committed(node) && !raft_node_is_voting_committed(node) &&
raft_node_is_addition_committed(node) &&
me->cb.node_has_sufficient_logs && me->cb.node_has_sufficient_logs &&
0 == raft_node_has_sufficient_logs(node) 0 == raft_node_has_sufficient_logs(node)
) )
......
...@@ -2704,6 +2704,7 @@ void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_for_nod ...@@ -2704,6 +2704,7 @@ void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_for_nod
aer.current_idx = 2; aer.current_idx = 2;
raft_node_set_voting(node, 0); raft_node_set_voting(node, 0);
raft_node_set_addition_committed(node, 1);
raft_recv_appendentries_response(r, node, &aer); raft_recv_appendentries_response(r, node, &aer);
CuAssertIntEquals(tc, 1, has_sufficient_logs_flag); CuAssertIntEquals(tc, 1, has_sufficient_logs_flag);
...@@ -2711,6 +2712,49 @@ void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_for_nod ...@@ -2711,6 +2712,49 @@ void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_for_nod
CuAssertIntEquals(tc, 1, has_sufficient_logs_flag); CuAssertIntEquals(tc, 1, has_sufficient_logs_flag);
} }
void TestRaft_leader_recv_appendentries_response_fail_set_has_sufficient_logs_for_node_if_not_addition_committed(
CuTest * tc)
{
raft_cbs_t funcs = {
.applylog = __raft_applylog,
.persist_term = __raft_persist_term,
.node_has_sufficient_logs = __raft_node_has_sufficient_logs,
.log = NULL
};
msg_appendentries_response_t aer;
void *r = raft_new();
raft_add_node(r, NULL, 1, 1);
raft_add_node(r, NULL, 2, 0);
raft_add_node(r, NULL, 3, 0);
raft_add_node(r, NULL, 4, 0);
raft_node_t* node = raft_add_node(r, NULL, 5, 0);
int has_sufficient_logs_flag = 0;
raft_set_callbacks(r, &funcs, &has_sufficient_logs_flag);
/* I'm the leader */
raft_set_state(r, RAFT_STATE_LEADER);
raft_set_current_term(r, 1);
raft_set_commit_idx(r, 0);
/* the last applied idx will became 1, and then 2 */
raft_set_last_applied_idx(r, 0);
/* append entries - we need two */
__RAFT_APPEND_ENTRIES_SEQ_ID(r, 3, 1, 1, "aaaa");
memset(&aer, 0, sizeof(msg_appendentries_response_t));
raft_send_appendentries(r, raft_get_node(r, 5));
aer.term = 1;
aer.success = 1;
aer.current_idx = 2;
raft_node_set_voting(node, 0);
raft_recv_appendentries_response(r, node, &aer);
CuAssertIntEquals(tc, 0, has_sufficient_logs_flag);
}
void TestRaft_leader_recv_appendentries_response_increase_commit_idx_using_voting_nodes_majority( void TestRaft_leader_recv_appendentries_response_increase_commit_idx_using_voting_nodes_majority(
CuTest * tc) CuTest * tc)
{ {
...@@ -3609,6 +3653,7 @@ void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_after_v ...@@ -3609,6 +3653,7 @@ void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_after_v
}; };
/* node 3 responds so it has sufficient logs and will be promoted */ /* node 3 responds so it has sufficient logs and will be promoted */
raft_node_set_addition_committed(raft_get_node(r, 3), 1);
raft_recv_appendentries_response(r, raft_get_node(r, 3), &aer); raft_recv_appendentries_response(r, raft_get_node(r, 3), &aer);
CuAssertIntEquals(tc, 1, has_sufficient_logs_flag); CuAssertIntEquals(tc, 1, has_sufficient_logs_flag);
...@@ -3617,6 +3662,7 @@ void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_after_v ...@@ -3617,6 +3662,7 @@ void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_after_v
raft_recv_entry(r, ety, &etyr); raft_recv_entry(r, ety, &etyr);
/* we now get a response from node 2, but it's still behind */ /* we now get a response from node 2, but it's still behind */
raft_node_set_addition_committed(raft_get_node(r, 2), 1);
raft_recv_appendentries_response(r, raft_get_node(r, 2), &aer); raft_recv_appendentries_response(r, raft_get_node(r, 2), &aer);
CuAssertIntEquals(tc, 1, has_sufficient_logs_flag); CuAssertIntEquals(tc, 1, has_sufficient_logs_flag);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment