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

Term changes must be persisted to disk. #2209

Closed
wants to merge 1 commit into from

Conversation

cannium
Copy link
Contributor

@cannium cannium commented Apr 9, 2015

No description provided.

@otoolep
Copy link
Contributor

otoolep commented Apr 9, 2015

Thanks @cannium -- good catch.

Reviewed by @benbjohnson -- merging on green. I had to kick the build due to a known issue with Continuous Queries.

@@ -959,7 +959,7 @@ func (l *Log) candidateLoop(closing <-chan struct{}) State {

// Increment term and request votes.
l.mu.Lock()
l.term++
l.mustSetTerm(l.term+1)
l.votedFor = l.id
term := l.term
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not actually quite right, since l.term is no longer being incremented.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, mustSetTerm takes care of setting l.term so we're actually OK.

@otoolep
Copy link
Contributor

otoolep commented Apr 9, 2015

Closed in favour of #2219 as I need to make more changes to the branch.

@otoolep otoolep closed this Apr 9, 2015
@otoolep
Copy link
Contributor

otoolep commented Apr 9, 2015

@cannium -- can you also sign the CLA? You can find it here:

http://influxdb.com/community/cla.html

@cannium
Copy link
Contributor Author

cannium commented Apr 10, 2015

Signed. Just curious, what's the difference between this patch and that of #2219 ?

@otoolep
Copy link
Contributor

otoolep commented Apr 10, 2015

The CHANGELOG. I needed this change in quickly to meet a build requirement. Otherwise I would have waited around, or added your repo as a remote, cherry-picked etc etc. Unfortunately I didn't have the time this time.

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.

2 participants