Snapshot can attempt to delete a negative range from log

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".

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"?

Please do take it! Thanks for contributing!

…

On Sun, Sep 8, 2019 at 10:19 AM jsjs026 ***@***.***> wrote: Are anyone on this one? If not may I take this as my first issue? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#358?email_source=notifications&email_token=AAA5QU3QNEOBM7AQO2D6N63QIS7RLA5CNFSM4IGTIUQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6FLTTA#issuecomment-529185228>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA5QU7CFPLSCEFRMOYVH7DQIS7RLANCNFSM4IGTIUQQ> .

#362 was opened, thanks @jscode017 :D

Can we close this issue as the fix is merged?

Thanks @dineshba good spot.

	// 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 {