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

Snapshot can attempt to delete a negative range from log #358

Closed
banks opened this issue Jul 24, 2019 · 5 comments
Closed

Snapshot can attempt to delete a negative range from log #358

banks opened this issue Jul 24, 2019 · 5 comments

Comments

@banks
Copy link
Member

banks commented Jul 24, 2019

Debugging something in consul I noticed raft log messages like this:

raft: Compacting logs from 26108 to 6079

Looking at

raft/snapshot.go

Lines 231 to 235 in ff523e1

// Log this
r.logger.Info(fmt.Sprintf("Compacting logs from %d to %d", minLog, maxLog))
// Compact the logs
if err := r.logs.DeleteRange(minLog, maxLog); err != nil {

This is in fact the min/max values we are passing to DeleteRange on the log store.

Our LogStore interface doesn't define what the behaviour should from implementations in that case although our raft-boltdb store that we use everywhere happens to make that a no-op so it's not causing actual issue other than a confusing log message.

I suggest we add a bounds check just before those lines to verify that max is strictly larger than min and if not just return possibly with a nice log like "no logs to truncate".

@jscode017
Copy link
Contributor

jscode017 commented Sep 8, 2019

Is anyone on this one?
If not may I take this as my first issue?
And do you mean "r.logger.Info("no logs to truncate")" and return nil, or just return "no logs to truncate"?

@banks
Copy link
Member Author

banks commented Sep 17, 2019 via email

@catsby
Copy link
Member

catsby commented Sep 18, 2019

#362 was opened, thanks @jscode017 :D

@dineshba
Copy link

dineshba commented Oct 1, 2019

Can we close this issue as the fix is merged?

@banks
Copy link
Member Author

banks commented Oct 1, 2019

Thanks @dineshba good spot.

@banks banks closed this as completed Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants