Skip to content

Commit

Permalink
Revert commits c306408 and da936a9.
Browse files Browse the repository at this point in the history
Turns out documentation is not up to date regarding the process of node
removal, and it the current implementation assumes a two-phase process
is always required (i.e. DEMOTE followed by REMOVE).
  • Loading branch information
yossigo committed Mar 23, 2020
1 parent 5d7915f commit 2c10536
Showing 1 changed file with 7 additions and 12 deletions.
19 changes: 7 additions & 12 deletions src/raft_server.c
Expand Up @@ -880,17 +880,6 @@ int raft_apply_entry(raft_server_t* me_)
log_idx, ety->id, ety->data_len);

me->last_applied_idx++;

/* voting cfg change is now complete. we have to update it before calling
* applylog to allow the callback to push a new membership change entry
* if needed (e.g. remove after demote).
*
* TODO: is there possibly an off-by-one bug hidden here? requiring
* checking log_idx >= voting_cfg_change_log_idx rather than plain ==.
*/
if (log_idx >= me->voting_cfg_change_log_idx)
me->voting_cfg_change_log_idx = -1;

if (me->cb.applylog)
{
int e = me->cb.applylog(me_, me->udata, ety, me->last_applied_idx);
Expand All @@ -900,6 +889,13 @@ int raft_apply_entry(raft_server_t* me_)
}
}

/* voting cfg change is now complete.
* TODO: there seem to be a possible off-by-one bug hidden here, requiring
* checking log_idx >= voting_cfg_change_log_idx rather than plain ==.
*/
if (log_idx >= me->voting_cfg_change_log_idx)
me->voting_cfg_change_log_idx = -1;

if (!raft_entry_is_cfg_change(ety))
goto exit;

Expand Down Expand Up @@ -1203,7 +1199,6 @@ int raft_apply_all(raft_server_t* me_)
int raft_entry_is_voting_cfg_change(raft_entry_t* ety)
{
return RAFT_LOGTYPE_ADD_NODE == ety->type ||
RAFT_LOGTYPE_REMOVE_NODE == ety->type ||
RAFT_LOGTYPE_DEMOTE_NODE == ety->type;
}

Expand Down

0 comments on commit 2c10536

Please sign in to comment.