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

Wait for quorum write before returning from Log.Apply(). #2566

Merged
merged 1 commit into from
May 13, 2015

Conversation

benbjohnson
Copy link
Contributor

Overview

This pull request ensures a commit is written to a quorum before returning from Log.Apply(). It also fixes issues with the raft.Clock to ensure that channels are closed when they're not needed and it also fixes some io.Closer objects that need to be closed.

@@ -341,9 +341,10 @@ func (c *Client) do(method, path string, values url.Values, contentType string,
// If a temporary redirect occurs then update the leader and retry.
// If a non-200 status is returned then an error occurred.
if resp.StatusCode == http.StatusTemporaryRedirect {
_ = resp.Body.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need the _ =?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. It's a habit I do from using errcheck on other projects. Removed in 00ce4a5.

This commit ensures a commit is written to a quorum before returning
from Log.Apply().
@@ -1024,6 +1030,9 @@ func (l *Log) candidateLoop(closing <-chan struct{}) State {
elected := make(chan struct{}, 1)
go l.elect(term, elected, &wg)

electionTimer := l.Clock.ElectionTimer()
defer electionTimer.Stop()
Copy link
Member

Choose a reason for hiding this comment

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

you're using defer here to stop the timer but not above. This intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The candidate loop doesn't reset the timer so it can just defer when it exits. The follower loop needs to manually close it wherever it exits the function since it can get reset.

@pauldix
Copy link
Member

pauldix commented May 13, 2015

LGTM, 🚢!!!

benbjohnson added a commit that referenced this pull request May 13, 2015
Wait for quorum write before returning from Log.Apply().
@benbjohnson benbjohnson merged commit d3823b3 into master May 13, 2015
@benbjohnson benbjohnson deleted the apply-wait branch May 13, 2015 22:33
@otoolep
Copy link
Contributor

otoolep commented May 14, 2015

I'm going to update the changelog for this.

otoolep added a commit that referenced this pull request May 15, 2015
This reverts commit d3823b3, reversing
changes made to 2de4bf2.
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.

4 participants