Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor raft state handling #1615

Merged
merged 7 commits into from
Feb 20, 2015
Merged

Refactor raft state handling #1615

merged 7 commits into from
Feb 20, 2015

Conversation

benbjohnson
Copy link
Contributor

Overview

This commit changes the state handling of the raft log. The actions for related to each raft state are strictly confined within that state's loop. To transition between states, the raft log now much clean up all its actions before moving on.

This fixes issues where goroutines were kicked off for one state but were delayed in their scheduling so they would begin after the log had already changed to another state.

This commit changes the state handling of the raft log. The actions
for related to each raft state are strictly confined within that state's
loop. To transition between states, the raft log now much clean up all
its actions before moving on.

This fixes issues where goroutines were kicked off for one state but
were delayed in their scheduling so they would begin after the log
had already changed to another state.
// AfterElectionTimeout returns a channel that fires after the election timeout.
func (c *Clock) AfterElectionTimeout() <-chan chan struct{} { return newClockChan(c.ElectionTimeout) }
// AfterElectionTimeout returns a channel that fires after a duration that is
// between the election timeout and double the election timeout.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches the Raft paper. If all the nodes return the same timeout then they tend to get stuck in an election loop indefinitely where they can't get enough votes.

@otoolep
Copy link
Contributor

otoolep commented Feb 17, 2015

This is a big change, difficult to grok it all. I'll take another pass after feedback and a green build.

@benbjohnson benbjohnson changed the title WIP: Refactor raft state handling Refactor raft state handling Feb 18, 2015
l.term = hb.term
l.votedFor = 0
}
if hb.commitIndex > l.commitIndex {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just so I am sure, here the index of the log is push forward to the index received over the heartbeat. I presume the entries corresponding to the greater commitIndex are also inserted into the log.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readFromLeader does it, I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A follower can get a commit index higher than it's uncommitted index if the replication lags behind. That's ok though. It'll just commit those entries when they come in.

@otoolep
Copy link
Contributor

otoolep commented Feb 19, 2015

OK, took a second pass. Still a big change. :-)

I do have some questions outstanding about the previous comments, I would like to get some feedback on those. I can follow at a very high-level what is going on, and the tests seem better. If you're happy I think you can merge.

benbjohnson added a commit that referenced this pull request Feb 20, 2015
Refactor raft state handling
@benbjohnson benbjohnson merged commit 2e32c31 into master Feb 20, 2015
@benbjohnson benbjohnson deleted the election branch February 20, 2015 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants