Commit 661032df authored by Willem Thiart's avatar Willem Thiart
Browse files

Fix current_idx management

The node tells the leader what's current_idx is using the appendentries
response.
parent fd1f0286
...@@ -46,7 +46,6 @@ void raft_node_set_next_idx(raft_node_t* me_, int nextIdx) ...@@ -46,7 +46,6 @@ void raft_node_set_next_idx(raft_node_t* me_, int nextIdx)
me->next_idx = nextIdx < 1 ? 1 : nextIdx; me->next_idx = nextIdx < 1 ? 1 : nextIdx;
} }
int raft_node_get_match_idx(raft_node_t* me_) int raft_node_get_match_idx(raft_node_t* me_)
{ {
raft_node_private_t* me = (raft_node_private_t*)me_; raft_node_private_t* me = (raft_node_private_t*)me_;
...@@ -59,7 +58,6 @@ void raft_node_set_match_idx(raft_node_t* me_, int matchIdx) ...@@ -59,7 +58,6 @@ void raft_node_set_match_idx(raft_node_t* me_, int matchIdx)
me->match_idx = matchIdx; me->match_idx = matchIdx;
} }
void* raft_node_get_udata(raft_node_t* me_) void* raft_node_get_udata(raft_node_t* me_)
{ {
raft_node_private_t* me = (raft_node_private_t*)me_; raft_node_private_t* me = (raft_node_private_t*)me_;
......
...@@ -95,7 +95,7 @@ void raft_become_leader(raft_server_t* me_) ...@@ -95,7 +95,7 @@ void raft_become_leader(raft_server_t* me_)
if (me->nodeid != i) if (me->nodeid != i)
{ {
raft_node_t* node = raft_get_node(me_, i); raft_node_t* node = raft_get_node(me_, i);
raft_node_set_next_idx(node, raft_get_current_idx(me_)); raft_node_set_next_idx(node, raft_get_current_idx(me_) + 1);
raft_node_set_match_idx(node, 0); raft_node_set_match_idx(node, 0);
raft_send_appendentries(me_, i); raft_send_appendentries(me_, i);
} }
...@@ -192,13 +192,21 @@ int raft_recv_appendentries_response(raft_server_t* me_, ...@@ -192,13 +192,21 @@ int raft_recv_appendentries_response(raft_server_t* me_,
/* If AppendEntries fails because of log inconsistency: /* If AppendEntries fails because of log inconsistency:
decrement nextIndex and retry (§5.3) */ decrement nextIndex and retry (§5.3) */
assert(0 <= raft_node_get_next_idx(node)); assert(0 <= raft_node_get_next_idx(node));
raft_node_set_next_idx(node, r->current_idx + 1);
int next_idx = raft_node_get_next_idx(node);
assert(0 <= next_idx);
if (r->current_idx < next_idx - 1)
raft_node_set_next_idx(node, min(r->current_idx + 1, raft_get_current_idx(me_)));
else
raft_node_set_next_idx(node, next_idx - 1);
/* retry */ /* retry */
raft_send_appendentries(me_, node_idx); raft_send_appendentries(me_, node_idx);
return 0; return 0;
} }
assert(r->current_idx <= raft_get_current_idx(me_));
/* response to a repeat transmission -- ignore */ /* response to a repeat transmission -- ignore */
if (raft_node_get_match_idx(node) == r->current_idx) if (raft_node_get_match_idx(node) == r->current_idx)
return 0; return 0;
...@@ -258,7 +266,7 @@ int raft_recv_appendentries( ...@@ -258,7 +266,7 @@ int raft_recv_appendentries(
if (ae->term < me->current_term) if (ae->term < me->current_term)
{ {
__log(me_, "AE term is less than current term"); __log(me_, "AE term is less than current term");
goto fail; goto fail_with_current_idx;
} }
/* Not the first appendentries we've received */ /* Not the first appendentries we've received */
...@@ -270,15 +278,22 @@ int raft_recv_appendentries( ...@@ -270,15 +278,22 @@ int raft_recv_appendentries(
if (!e) if (!e)
{ {
__log(me_, "AE no log at prev_idx %d", ae->prev_log_idx); __log(me_, "AE no log at prev_idx %d", ae->prev_log_idx);
goto fail; goto fail_with_current_idx;
} }
/* 2. Reply false if log doesn't contain an entry at prevLogIndex /* 2. Reply false if log doesn't contain an entry at prevLogIndex
whose term matches prevLogTerm (§5.3) */ whose term matches prevLogTerm (§5.3) */
if (raft_get_current_idx(me_) < ae->prev_log_idx)
goto fail_with_current_idx;
if (e->term != ae->prev_log_term) if (e->term != ae->prev_log_term)
{ {
__log(me_, "AE term doesn't match prev_idx (ie. %d vs %d)", __log(me_, "AE term doesn't match prev_idx (ie. %d vs %d)",
e->term, ae->prev_log_term); e->term, ae->prev_log_term);
assert(me->commit_idx < ae->prev_log_idx);
/* Delete all the following log entries because they don't match */
log_delete(me->log, ae->prev_log_idx);
r->current_idx = ae->prev_log_idx - 1;
goto fail; goto fail;
} }
} }
...@@ -286,14 +301,24 @@ int raft_recv_appendentries( ...@@ -286,14 +301,24 @@ int raft_recv_appendentries(
/* 3. If an existing entry conflicts with a new one (same index /* 3. If an existing entry conflicts with a new one (same index
but different terms), delete the existing entry and all that but different terms), delete the existing entry and all that
follow it (§5.3) */ follow it (§5.3) */
if (ae->n_entries == 0 && 0 < ae->prev_log_idx && ae->prev_log_idx + 1 < raft_get_current_idx(me_))
{
assert(me->commit_idx < ae->prev_log_idx + 1);
log_delete(me->log, ae->prev_log_idx + 1);
}
r->current_idx = ae->prev_log_idx;
int i; int i;
for (i = 0; i < ae->n_entries; i++) for (i = 0; i < ae->n_entries; i++)
{ {
msg_entry_t* ety = &ae->entries[i]; msg_entry_t* ety = &ae->entries[i];
int ety_index = ae->prev_log_idx + 1 + i; int ety_index = ae->prev_log_idx + 1 + i;
raft_entry_t* existing_ety = raft_get_entry_from_idx(me_, ety_index); raft_entry_t* existing_ety = raft_get_entry_from_idx(me_, ety_index);
r->current_idx = ety_index;
if (existing_ety && existing_ety->term != ety->term) if (existing_ety && existing_ety->term != ety->term)
{ {
assert(me->commit_idx < ety_index);
log_delete(me->log, ety_index); log_delete(me->log, ety_index);
break; break;
} }
...@@ -306,7 +331,9 @@ int raft_recv_appendentries( ...@@ -306,7 +331,9 @@ int raft_recv_appendentries(
{ {
int e = raft_append_entry(me_, &ae->entries[i]); int e = raft_append_entry(me_, &ae->entries[i]);
if (-1 == e) if (-1 == e)
goto fail; goto fail_with_current_idx;
r->current_idx = ae->prev_log_idx + 1 + i;
} }
/* 4. If leaderCommit > commitIndex, set commitIndex = /* 4. If leaderCommit > commitIndex, set commitIndex =
...@@ -321,13 +348,13 @@ int raft_recv_appendentries( ...@@ -321,13 +348,13 @@ int raft_recv_appendentries(
me->current_leader = node; me->current_leader = node;
r->success = 1; r->success = 1;
r->current_idx = raft_get_current_idx(me_);
r->first_idx = ae->prev_log_idx + 1; r->first_idx = ae->prev_log_idx + 1;
return 0; return 0;
fail_with_current_idx:
r->current_idx = raft_get_current_idx(me_);
fail: fail:
r->success = 0; r->success = 0;
r->current_idx = raft_get_current_idx(me_);
r->first_idx = 0; r->first_idx = 0;
return -1; return -1;
} }
......
...@@ -746,24 +746,16 @@ void TestRaft_follower_recv_appendentries_reply_false_if_doesnt_have_log_at_prev ...@@ -746,24 +746,16 @@ void TestRaft_follower_recv_appendentries_reply_false_if_doesnt_have_log_at_prev
CuAssertTrue(tc, 0 == aer.success); CuAssertTrue(tc, 0 == aer.success);
} }
/* 5.3 */ static raft_entry_t* __entries_for_conflict_tests(
void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_entries( CuTest * tc,
CuTest * tc) raft_server_t* r,
char** strs)
{ {
msg_appendentries_t ae;
msg_appendentries_response_t aer;
raft_entry_t *ety_appended;
void *r = raft_new();
raft_add_node(r, (void*)1, 1);
raft_add_node(r, (void*)2, 0);
raft_set_current_term(r, 1);
raft_entry_t ety; raft_entry_t ety;
raft_entry_t *ety_appended;
/* increase log size */ /* increase log size */
char *str1 = "111"; char *str1 = strs[0];
ety.data.buf = str1; ety.data.buf = str1;
ety.data.len = 3; ety.data.len = 3;
ety.id = 1; ety.id = 1;
...@@ -772,7 +764,7 @@ void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_en ...@@ -772,7 +764,7 @@ void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_en
CuAssertTrue(tc, 1 == raft_get_log_count(r)); CuAssertTrue(tc, 1 == raft_get_log_count(r));
/* this log will be overwritten by the appendentries below */ /* this log will be overwritten by the appendentries below */
char *str2 = "222"; char *str2 = strs[1];
ety.data.buf = str2; ety.data.buf = str2;
ety.data.len = 3; ety.data.len = 3;
ety.id = 2; ety.id = 2;
...@@ -783,7 +775,7 @@ void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_en ...@@ -783,7 +775,7 @@ void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_en
CuAssertTrue(tc, !strncmp(ety_appended->data.buf, str2, 3)); CuAssertTrue(tc, !strncmp(ety_appended->data.buf, str2, 3));
/* this log will be overwritten by the appendentries below */ /* this log will be overwritten by the appendentries below */
char *str3 = "333"; char *str3 = strs[2];
ety.data.buf = str3; ety.data.buf = str3;
ety.data.len = 3; ety.data.len = 3;
ety.id = 3; ety.id = 3;
...@@ -793,6 +785,25 @@ void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_en ...@@ -793,6 +785,25 @@ void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_en
CuAssertTrue(tc, NULL != (ety_appended = raft_get_entry_from_idx(r, 3))); CuAssertTrue(tc, NULL != (ety_appended = raft_get_entry_from_idx(r, 3)));
CuAssertTrue(tc, !strncmp(ety_appended->data.buf, str3, 3)); CuAssertTrue(tc, !strncmp(ety_appended->data.buf, str3, 3));
return ety_appended;
}
/* 5.3 */
void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_entries(
CuTest * tc)
{
msg_appendentries_t ae;
msg_appendentries_response_t aer;
void *r = raft_new();
raft_add_node(r, (void*)1, 1);
raft_add_node(r, (void*)2, 0);
raft_set_current_term(r, 1);
char* strs[] = {"111", "222", "333"};
raft_entry_t *ety_appended = __entries_for_conflict_tests(tc, r, strs);
/* pass a appendentry that is newer */ /* pass a appendentry that is newer */
msg_entry_t mety; msg_entry_t mety;
...@@ -813,7 +824,36 @@ void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_en ...@@ -813,7 +824,36 @@ void TestRaft_follower_recv_appendentries_delete_entries_if_conflict_with_new_en
CuAssertTrue(tc, 1 == aer.success); CuAssertTrue(tc, 1 == aer.success);
CuAssertTrue(tc, 2 == raft_get_log_count(r)); CuAssertTrue(tc, 2 == raft_get_log_count(r));
CuAssertTrue(tc, NULL != (ety_appended = raft_get_entry_from_idx(r, 1))); CuAssertTrue(tc, NULL != (ety_appended = raft_get_entry_from_idx(r, 1)));
CuAssertTrue(tc, !strncmp(ety_appended->data.buf, str1, 3)); CuAssertTrue(tc, !strncmp(ety_appended->data.buf, strs[0], 3));
}
void TestRaft_follower_recv_appendentries_delete_entries_if_current_idx_greater_than_prev_log_idx(
CuTest * tc)
{
msg_appendentries_t ae;
msg_appendentries_response_t aer;
void *r = raft_new();
raft_add_node(r, (void*)1, 1);
raft_add_node(r, (void*)2, 0);
raft_set_current_term(r, 1);
char* strs[] = {"111", "222", "333"};
raft_entry_t *ety_appended = __entries_for_conflict_tests(tc, r, strs);
memset(&ae, 0, sizeof(msg_appendentries_t));
ae.term = 2;
ae.prev_log_idx = 1;
ae.prev_log_term = 1;
ae.entries = NULL;
ae.n_entries = 0;
raft_recv_appendentries(r, 1, &ae, &aer);
CuAssertTrue(tc, 1 == aer.success);
CuAssertTrue(tc, 1 == raft_get_log_count(r));
CuAssertTrue(tc, NULL != (ety_appended = raft_get_entry_from_idx(r, 1)));
CuAssertTrue(tc, !strncmp(ety_appended->data.buf, strs[0], 3));
} }
void TestRaft_follower_recv_appendentries_add_new_entries_not_already_in_log( void TestRaft_follower_recv_appendentries_add_new_entries_not_already_in_log(
...@@ -1783,7 +1823,7 @@ void TestRaft_leader_recv_appendentries_response_jumps_to_lower_next_idx( ...@@ -1783,7 +1823,7 @@ void TestRaft_leader_recv_appendentries_response_jumps_to_lower_next_idx(
raft_set_current_term(r, 2); raft_set_current_term(r, 2);
raft_set_commit_idx(r, 0); raft_set_commit_idx(r, 0);
/* append entries - we need two */ /* append entries */
raft_entry_t ety; raft_entry_t ety;
ety.term = 1; ety.term = 1;
ety.id = 1; ety.id = 1;
...@@ -1805,7 +1845,7 @@ void TestRaft_leader_recv_appendentries_response_jumps_to_lower_next_idx( ...@@ -1805,7 +1845,7 @@ void TestRaft_leader_recv_appendentries_response_jumps_to_lower_next_idx(
/* become leader sets next_idx to current_idx */ /* become leader sets next_idx to current_idx */
raft_become_leader(r); raft_become_leader(r);
raft_node_t* node = raft_get_node(r, 1); raft_node_t* node = raft_get_node(r, 1);
CuAssertIntEquals(tc, 4, raft_node_get_next_idx(node)); CuAssertIntEquals(tc, 5, raft_node_get_next_idx(node));
CuAssertTrue(tc, NULL != (ae = sender_poll_msg_data(sender))); CuAssertTrue(tc, NULL != (ae = sender_poll_msg_data(sender)));
/* FIRST entry log application */ /* FIRST entry log application */
...@@ -1813,8 +1853,8 @@ void TestRaft_leader_recv_appendentries_response_jumps_to_lower_next_idx( ...@@ -1813,8 +1853,8 @@ void TestRaft_leader_recv_appendentries_response_jumps_to_lower_next_idx(
* server will be waiting for response */ * server will be waiting for response */
raft_send_appendentries(r, 1); raft_send_appendentries(r, 1);
CuAssertTrue(tc, NULL != (ae = sender_poll_msg_data(sender))); CuAssertTrue(tc, NULL != (ae = sender_poll_msg_data(sender)));
CuAssertIntEquals(tc, 3, ae->prev_log_term); CuAssertIntEquals(tc, 4, ae->prev_log_term);
CuAssertIntEquals(tc, 3, ae->prev_log_idx); CuAssertIntEquals(tc, 4, ae->prev_log_idx);
/* receive mock success responses */ /* receive mock success responses */
memset(&aer, 0, sizeof(msg_appendentries_response_t)); memset(&aer, 0, sizeof(msg_appendentries_response_t));
...@@ -1832,6 +1872,86 @@ void TestRaft_leader_recv_appendentries_response_jumps_to_lower_next_idx( ...@@ -1832,6 +1872,86 @@ void TestRaft_leader_recv_appendentries_response_jumps_to_lower_next_idx(
CuAssertTrue(tc, NULL == sender_poll_msg_data(sender)); CuAssertTrue(tc, NULL == sender_poll_msg_data(sender));
} }
void TestRaft_leader_recv_appendentries_response_decrements_to_lower_next_idx(
CuTest * tc)
{
raft_cbs_t funcs = {
.send_appendentries = sender_appendentries,
.log = NULL
};
msg_appendentries_response_t aer;
void *sender = sender_new(NULL);
void *r = raft_new();
raft_add_node(r, (void*)1, 1);
raft_add_node(r, (void*)2, 0);
raft_set_callbacks(r, &funcs, sender);
raft_set_current_term(r, 2);
raft_set_commit_idx(r, 0);
/* append entries */
raft_entry_t ety;
ety.term = 1;
ety.id = 1;
ety.data.buf = "aaaa";
ety.data.len = 4;
raft_append_entry(r, &ety);
ety.term = 2;
ety.id = 2;
raft_append_entry(r, &ety);
ety.term = 3;
ety.id = 3;
raft_append_entry(r, &ety);
ety.term = 4;
ety.id = 4;
raft_append_entry(r, &ety);
msg_appendentries_t* ae;
/* become leader sets next_idx to current_idx */
raft_become_leader(r);
raft_node_t* node = raft_get_node(r, 1);
CuAssertIntEquals(tc, 5, raft_node_get_next_idx(node));
CuAssertTrue(tc, NULL != (ae = sender_poll_msg_data(sender)));
/* FIRST entry log application */
/* send appendentries -
* server will be waiting for response */
raft_send_appendentries(r, 1);
CuAssertTrue(tc, NULL != (ae = sender_poll_msg_data(sender)));
CuAssertIntEquals(tc, 4, ae->prev_log_term);
CuAssertIntEquals(tc, 4, ae->prev_log_idx);
/* receive mock success responses */
memset(&aer, 0, sizeof(msg_appendentries_response_t));
aer.term = 2;
aer.success = 0;
aer.current_idx = 4;
raft_recv_appendentries_response(r, 1, &aer);
CuAssertIntEquals(tc, 4, raft_node_get_next_idx(node));
/* see if new appendentries have appropriate values */
CuAssertTrue(tc, NULL != (ae = sender_poll_msg_data(sender)));
CuAssertIntEquals(tc, 3, ae->prev_log_term);
CuAssertIntEquals(tc, 3, ae->prev_log_idx);
/* receive mock success responses */
memset(&aer, 0, sizeof(msg_appendentries_response_t));
aer.term = 2;
aer.success = 0;
aer.current_idx = 4;
raft_recv_appendentries_response(r, 1, &aer);
CuAssertIntEquals(tc, 3, raft_node_get_next_idx(node));
/* see if new appendentries have appropriate values */
CuAssertTrue(tc, NULL != (ae = sender_poll_msg_data(sender)));
CuAssertIntEquals(tc, 2, ae->prev_log_term);
CuAssertIntEquals(tc, 2, ae->prev_log_idx);
CuAssertTrue(tc, NULL == sender_poll_msg_data(sender));
}
void TestRaft_leader_recv_appendentries_response_retry_only_if_leader(CuTest * tc) void TestRaft_leader_recv_appendentries_response_retry_only_if_leader(CuTest * tc)
{ {
raft_cbs_t funcs = { raft_cbs_t funcs = {
...@@ -2080,16 +2200,16 @@ void TestRaft_leader_recv_appendentries_response_increment_idx_of_node( ...@@ -2080,16 +2200,16 @@ void TestRaft_leader_recv_appendentries_response_increment_idx_of_node(
raft_set_current_term(r, 1); raft_set_current_term(r, 1);
raft_node_t* p = raft_get_node(r, 1); raft_node_t* p = raft_get_node(r, 1);
CuAssertTrue(tc, 1 == raft_node_get_next_idx(p)); CuAssertIntEquals(tc, 1, raft_node_get_next_idx(p));
/* receive mock success responses */ /* receive mock success responses */
msg_appendentries_response_t aer; msg_appendentries_response_t aer;
aer.term = 1; aer.term = 1;
aer.success = 1; aer.success = 1;
aer.current_idx = 1; aer.current_idx = 0;
aer.first_idx = 1; aer.first_idx = 0;
raft_recv_appendentries_response(r, 1, &aer); raft_recv_appendentries_response(r, 1, &aer);
CuAssertTrue(tc, 2 == raft_node_get_next_idx(p)); CuAssertIntEquals(tc, 1, raft_node_get_next_idx(p));
} }
void TestRaft_leader_recv_appendentries_response_drop_message_if_term_is_old( void TestRaft_leader_recv_appendentries_response_drop_message_if_term_is_old(
......
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