-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Conversation
@@ -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() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need the _ =
?
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
LGTM, 🚢!!! |
Wait for quorum write before returning from Log.Apply().
I'm going to update the changelog for this. |
Overview
This pull request ensures a commit is written to a quorum before returning from
Log.Apply()
. It also fixes issues with theraft.Clock
to ensure that channels are closed when they're not needed and it also fixes someio.Closer
objects that need to be closed.