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

Set last contact on leader step down #83

Merged
merged 1 commit into from
Jan 22, 2016
Merged

Set last contact on leader step down #83

merged 1 commit into from
Jan 22, 2016

Conversation

armon
Copy link
Member

@armon armon commented Jan 20, 2016

Currently when the leader steps down, its last contact time will be from when it was last contacted as a follower, which may be either zero or a very long time ago. This fix updates the last contact time to when we stepped down, better reflecting the recency of the data.

@@ -792,6 +799,11 @@ func (r *Raft) runLeader() {
select {
case notify <- false:
case <-r.shutdownCh:
// On shutdown, make a best effort but do not block
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this related to the last contact change or something else?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its a race condition in one of the unit tests, basically during a shutdown we may fail to push to notify channel, so this tries to fix it without blocking.

@slackpad
Copy link
Contributor

@armon noted two small comments, otherwise looks good.

Took a few CI pokes to get things to pass (especially with the ancient Go version) but I don't think those failures were related to your change.

@superfell
Copy link
Contributor

Looks good to me.

armon added a commit that referenced this pull request Jan 22, 2016
Set last contact on leader step down
@armon armon merged commit 057b893 into master Jan 22, 2016
@armon armon deleted the f-lastcontact branch January 22, 2016 00:30
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.

3 participants