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

match up raft_get_last_log_term() to should_grant_vote() (#14)

raft_get_last_log_term() is used for filling in vote requests, except it didn't match 100% with the logic in should_grant_vote

* update test to prove that change works as needed

previous test tested when we don't snapshot everything, but code was broken when we did.
modified test tested only when we snapshot everything, now we do both in sequence.

* use raft_get_last_log_term in __should_grant_vote()

also modify __should_grant_vote to take a raft_server_t instead of raft_server_privat_t so its not constantly casting it to void.
parent 600cf4fe
...@@ -38,6 +38,7 @@ clinkedlistqueue: ...@@ -38,6 +38,7 @@ clinkedlistqueue:
download-contrib: clinkedlistqueue download-contrib: clinkedlistqueue
.PHONY: $(TEST_DIR)/main_test.c
$(TEST_DIR)/main_test.c: $(TEST_DIR)/main_test.c:
if test -d $(LLQUEUE_DIR); \ if test -d $(LLQUEUE_DIR); \
then echo have contribs; \ then echo have contribs; \
......
...@@ -579,37 +579,27 @@ int raft_already_voted(raft_server_t* me_) ...@@ -579,37 +579,27 @@ int raft_already_voted(raft_server_t* me_)
return ((raft_server_private_t*)me_)->voted_for != -1; return ((raft_server_private_t*)me_)->voted_for != -1;
} }
static int __should_grant_vote(raft_server_private_t* me, msg_requestvote_t* vr) static int __should_grant_vote(raft_server_t* me_, msg_requestvote_t* vr)
{ {
if (!raft_node_is_voting(raft_get_my_node((void*)me))) if (!raft_node_is_voting(raft_get_my_node(me_)))
return 0; return 0;
if (vr->term < raft_get_current_term((void*)me)) if (vr->term < raft_get_current_term(me_))
return 0; return 0;
/* TODO: if voted for is candidate return 1 (if below checks pass) */ /* TODO: if voted for is candidate return 1 (if below checks pass) */
if (raft_already_voted((void*)me)) if (raft_already_voted(me_))
return 0; return 0;
/* Below we check if log is more up-to-date... */ /* Below we check if log is more up-to-date... */
raft_index_t current_idx = raft_get_current_idx((void*)me); raft_index_t current_idx = raft_get_current_idx(me_);
/* Our log is definitely not more up-to-date if it's empty! */ /* Our log is definitely not more up-to-date if it's empty! */
if (0 == current_idx) if (0 == current_idx)
return 1; return 1;
raft_entry_t* ety = raft_get_entry_from_idx((void*)me, current_idx); raft_term_t ety_term = raft_get_last_log_term(me_);
raft_term_t ety_term;
// TODO: add test
if (ety) {
ety_term = ety->term;
raft_entry_release(ety);
} else if (!ety && me->snapshot_last_idx == current_idx)
ety_term = me->snapshot_last_term;
else
return 0;
if (ety_term < vr->last_log_term) if (ety_term < vr->last_log_term)
return 1; return 1;
...@@ -649,7 +639,7 @@ int raft_recv_requestvote(raft_server_t* me_, ...@@ -649,7 +639,7 @@ int raft_recv_requestvote(raft_server_t* me_,
me->current_leader = NULL; me->current_leader = NULL;
} }
if (__should_grant_vote(me, vr)) if (__should_grant_vote(me_, vr))
{ {
/* It shouldn't be possible for a leader or candidate to grant a vote /* It shouldn't be possible for a leader or candidate to grant a vote
* Both states would have voted for themselves */ * Both states would have voted for themselves */
......
...@@ -223,8 +223,11 @@ raft_term_t raft_get_last_log_term(raft_server_t* me_) ...@@ -223,8 +223,11 @@ raft_term_t raft_get_last_log_term(raft_server_t* me_)
raft_term_t term = ety->term; raft_term_t term = ety->term;
raft_entry_release(ety); raft_entry_release(ety);
return term; return term;
} else if (raft_get_snapshot_last_idx(me_) == current_idx) {
return raft_get_snapshot_last_term(me_);
} }
} }
return 0; return 0;
} }
......
...@@ -237,12 +237,14 @@ void TestRaft_leader_snapshot_end_succeeds_if_log_compacted(CuTest * tc) ...@@ -237,12 +237,14 @@ void TestRaft_leader_snapshot_end_succeeds_if_log_compacted(CuTest * tc)
raft_set_commit_idx(r, 2); raft_set_commit_idx(r, 2);
CuAssertIntEquals(tc, 3, raft_get_log_count(r)); CuAssertIntEquals(tc, 3, raft_get_log_count(r));
CuAssertIntEquals(tc, 2, raft_get_num_snapshottable_logs(r)); CuAssertIntEquals(tc, 2, raft_get_num_snapshottable_logs(r));
CuAssertIntEquals(tc, 1, raft_get_last_log_term(r));
CuAssertIntEquals(tc, 0, raft_begin_snapshot(r, 0)); CuAssertIntEquals(tc, 0, raft_begin_snapshot(r, 0));
int i = raft_get_first_entry_idx(r); int i = raft_get_first_entry_idx(r);
for (; i < raft_get_commit_idx(r); i++) for (; i < raft_get_commit_idx(r); i++) {
CuAssertIntEquals(tc, 0, raft_poll_entry(r)); CuAssertIntEquals(tc, 0, raft_poll_entry(r));
}
CuAssertIntEquals(tc, 0, raft_begin_snapshot(r, 0)); CuAssertIntEquals(tc, 0, raft_begin_snapshot(r, 0));
CuAssertIntEquals(tc, 0, raft_end_snapshot(r)); CuAssertIntEquals(tc, 0, raft_end_snapshot(r));
...@@ -250,7 +252,37 @@ void TestRaft_leader_snapshot_end_succeeds_if_log_compacted(CuTest * tc) ...@@ -250,7 +252,37 @@ void TestRaft_leader_snapshot_end_succeeds_if_log_compacted(CuTest * tc)
CuAssertIntEquals(tc, 1, raft_get_log_count(r)); CuAssertIntEquals(tc, 1, raft_get_log_count(r));
CuAssertIntEquals(tc, 2, raft_get_commit_idx(r)); CuAssertIntEquals(tc, 2, raft_get_commit_idx(r));
CuAssertIntEquals(tc, 2, raft_get_last_applied_idx(r)); CuAssertIntEquals(tc, 2, raft_get_last_applied_idx(r));
CuAssertIntEquals(tc, 1, raft_get_last_log_term(r));
CuAssertIntEquals(tc, 0, raft_periodic(r, 1000)); CuAssertIntEquals(tc, 0, raft_periodic(r, 1000));
/* the above test returns correct term as didn't snapshot all entries, makes sure works if we snapshot all */
/* snapshot doesn't work if only one entry to snapshot, so add another one */
ety = __MAKE_ENTRY(4, 1, "entry");
raft_recv_entry(r, ety, &cr);
raft_set_commit_idx(r, 4);
CuAssertIntEquals(tc, 0, raft_periodic(r, 1000));
CuAssertIntEquals(tc, 2, raft_get_log_count(r));
CuAssertIntEquals(tc, 4, raft_get_commit_idx(r));
raft_server_private_t *r_p = (raft_server_private_t *) r;
CuAssertIntEquals(tc, 3, r_p->log_impl->first_idx(r_p->log));
CuAssertIntEquals(tc, 2, raft_get_num_snapshottable_logs(r));
i = raft_get_first_entry_idx(r);
for (; i < raft_get_commit_idx(r); i++) {
printf("raft_poll_entry(2): %d\n", i);
CuAssertIntEquals(tc, 0, raft_poll_entry(r));
}
CuAssertIntEquals(tc, 0, raft_begin_snapshot(r, 0));
CuAssertIntEquals(tc, 0, raft_end_snapshot(r));
CuAssertIntEquals(tc, 0, raft_get_num_snapshottable_logs(r));
CuAssertIntEquals(tc, 0, raft_get_log_count(r));
CuAssertIntEquals(tc, 4, raft_get_commit_idx(r));
CuAssertIntEquals(tc, 4, raft_get_last_applied_idx(r));
CuAssertIntEquals(tc, 1, raft_get_last_log_term(r));
} }
void TestRaft_leader_snapshot_end_succeeds_if_log_compacted2(CuTest * tc) void TestRaft_leader_snapshot_end_succeeds_if_log_compacted2(CuTest * tc)
......
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