Raft config update fix #364
Conversation
Currently if node is no longer in configuration it may be promoted to voter as `raft::configuration::is_voter()` return `false` when node is not present in configuration - this is correct as node is not a voter. Added explicit check to make sure that node is part of current configuration before promoting it from learner to voter. Signed-off-by: Michal Maslanka <michal@vectorized.io>
Added updating `_target_priority` before skipping dispatching votes when node is not a voter. Learners are allowed to vote (not dispatch vote requests), since learner may not be updated with information that it was promoted while other nodes are. Learners ability to cast votes requires learners to update the `_target_priority` value every time leader election wasn't successful. The update was done when dispatching votes, but for learners dispatching vote was skipped, hence the `_target_priority` wasn't updated on learners. This might lead to situation where raft protocol couldn't make progress since leader election would never succeed. Fixed the issue by checking if node is a voter after updating `_target_priority`/ Signed-off-by: Michal Maslanka <michal@vectorized.io>
After fixing vote issues during configuration update, enabled back the `replace_whole_group` test. Signed-off-by: Michal Maslanka <michal@vectorized.io>
mmaslankaprv
force-pushed
the
raft-config-update-fix
branch
from
7beadf4
to
69cee3d
Compare
| // is voter already | ||
| if (config().is_voter(id)) { | ||
| if (latest_cfg.is_voter(id)) { |
There was a problem hiding this comment.
if it is possible that maybe_promote_to_voter is called for a node id that is already a voter, is it possible that it is called for a node id that is already a voter and is also not in the current configuration? would that need a special case like demotion to candidate?
There was a problem hiding this comment.
What do you mean by demotion to candidate ?
There was a problem hiding this comment.
looking back at this i don't think i have the same question now. i must have be confused about the combination of conditions.
| @@ -500,8 +500,6 @@ bool consensus::should_skip_vote(bool ignore_heartbeat) { | |||
| } | |||
|
|
|||
| skip_vote |= _vstate == vote_state::leader; // already a leader | |||
| skip_vote |= !_configuration_manager.get_latest().is_voter( | |||
There was a problem hiding this comment.
does this change mean that vote_stm::vote would now allow a non-voter cast a vote?
There was a problem hiding this comment.
non-voters were always allow to cast a vote but not to request a vote. This behavior didn't change
Fixed
replcate_whole_grouptest. The test failed because of an error in leader election process.Fixes: #342
Checklist
When referencing a related issue, remember to migrate duplicate stories from the
external tracker. This is not relevant for most users.