From 45e96be605c2b7f1d5777b85fa6fe20930c19397 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Fri, 14 Nov 2014 13:53:42 -0500 Subject: [PATCH] raft: PR feedback. Removed Get prefix in method names, added assertions and fixed comments. --- raft/log.go | 23 ++++++++++++++--------- raft/log_test.go | 4 ++-- raft/storage.go | 25 +++++++++++++------------ 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/raft/log.go b/raft/log.go index 1a16bb268..428b0396d 100644 --- a/raft/log.go +++ b/raft/log.go @@ -52,7 +52,7 @@ func newLog(storage Storage) *raftLog { log := &raftLog{ storage: storage, } - lastIndex, err := storage.GetLastIndex() + lastIndex, err := storage.LastIndex() if err == ErrStorageEmpty { // When starting from scratch populate the list with a dummy entry at term zero. log.unstableEnts = make([]pb.Entry, 1) @@ -94,6 +94,9 @@ func (l *raftLog) maybeAppend(index, logTerm, committed uint64, ents ...pb.Entry } func (l *raftLog) append(after uint64, ents ...pb.Entry) uint64 { + if after < l.committed { + log.Panicf("appending after %d, but already committed through %d", after, l.committed) + } if after < l.unstable { // The log is being truncated to before our current unstable // portion, so discard it and reset unstable. @@ -132,9 +135,7 @@ func (l *raftLog) unstableEntries() []pb.Entry { if len(l.unstableEnts) == 0 { return nil } - cpy := make([]pb.Entry, len(l.unstableEnts)) - copy(cpy, l.unstableEnts) - return cpy + return append([]pb.Entry(nil), l.unstableEnts...) } // nextEnts returns all the available entries for execution. @@ -147,7 +148,7 @@ func (l *raftLog) nextEnts() (ents []pb.Entry) { } func (l *raftLog) firstIndex() uint64 { - index, err := l.storage.GetFirstIndex() + index, err := l.storage.FirstIndex() if err != nil { panic(err) // TODO(bdarnell) } @@ -158,7 +159,7 @@ func (l *raftLog) lastIndex() uint64 { if len(l.unstableEnts) > 0 { return l.unstable + uint64(len(l.unstableEnts)) - 1 } - index, err := l.storage.GetLastIndex() + index, err := l.storage.LastIndex() if err != nil { panic(err) // TODO(bdarnell) } @@ -176,6 +177,10 @@ func (l *raftLog) appliedTo(i uint64) { } func (l *raftLog) stableTo(i uint64) { + if i < l.unstable || i+1-l.unstable > uint64(len(l.unstableEnts)) { + log.Panicf("stableTo(%d) is out of range (unstable=%d, len(unstableEnts)=%d)", + i, l.unstable, len(l.unstableEnts)) + } l.unstableEnts = l.unstableEnts[i+1-l.unstable:] l.unstable = i + 1 } @@ -247,11 +252,11 @@ func (l *raftLog) compact(i uint64) uint64 { panic(err) // TODO(bdarnell) } l.unstable = max(i+1, l.unstable) - firstIndex, err := l.storage.GetFirstIndex() + firstIndex, err := l.storage.FirstIndex() if err != nil { panic(err) // TODO(bdarnell) } - lastIndex, err := l.storage.GetLastIndex() + lastIndex, err := l.storage.LastIndex() if err != nil { panic(err) // TODO(bdarnell) } @@ -297,7 +302,7 @@ func (l *raftLog) slice(lo uint64, hi uint64) []pb.Entry { } var ents []pb.Entry if lo < l.unstable { - storedEnts, err := l.storage.GetEntries(lo, min(hi, l.unstable)) + storedEnts, err := l.storage.Entries(lo, min(hi, l.unstable)) if err != nil { panic(err) // TODO(bdarnell) } diff --git a/raft/log_test.go b/raft/log_test.go index 21ff06fad..27089d930 100644 --- a/raft/log_test.go +++ b/raft/log_test.go @@ -495,7 +495,7 @@ func TestAt(t *testing.T) { l := newLog(NewMemoryStorage()) l.restore(pb.Snapshot{Index: offset}) - for i = 0; i < num; i++ { + for i = 1; i < num; i++ { l.append(offset+i-1, pb.Entry{Term: i}) } @@ -525,7 +525,7 @@ func TestSlice(t *testing.T) { l := newLog(NewMemoryStorage()) l.restore(pb.Snapshot{Index: offset}) - for i = 0; i < num; i++ { + for i = 1; i < num; i++ { l.append(offset+i-1, pb.Entry{Term: i}) } diff --git a/raft/storage.go b/raft/storage.go index 7733b2ba0..049b9ea17 100644 --- a/raft/storage.go +++ b/raft/storage.go @@ -34,17 +34,18 @@ var ErrStorageEmpty = errors.New("storage is empty") // become inoperable and refuse to participate in elections; the // application is responsible for cleanup and recovery in this case. type Storage interface { - // GetEntries returns a slice of log entries in the range [lo,hi). - GetEntries(lo, hi uint64) ([]pb.Entry, error) + // Entries returns a slice of log entries in the range [lo,hi). + Entries(lo, hi uint64) ([]pb.Entry, error) // GetLastIndex returns the index of the last entry in the log. // If the log is empty it returns ErrStorageEmpty. - GetLastIndex() (uint64, error) + LastIndex() (uint64, error) // GetFirstIndex returns the index of the first log entry that is // available via GetEntries (older entries have been incorporated // into the latest Snapshot). - GetFirstIndex() (uint64, error) - // Compact discards all log entries prior to i, creating a snapshot - // which can be used to reconstruct the state at that point. + FirstIndex() (uint64, error) + // Compact discards all log entries prior to i. + // TODO(bdarnell): Create a snapshot which can be used to + // reconstruct the state at that point. Compact(i uint64) error } @@ -67,15 +68,15 @@ func NewMemoryStorage() *MemoryStorage { return &MemoryStorage{} } -// GetEntries implements the Storage interface. -func (ms *MemoryStorage) GetEntries(lo, hi uint64) ([]pb.Entry, error) { +// Entries implements the Storage interface. +func (ms *MemoryStorage) Entries(lo, hi uint64) ([]pb.Entry, error) { ms.Lock() defer ms.Unlock() return ms.ents[lo-ms.offset : hi-ms.offset], nil } -// GetLastIndex implements the Storage interface. -func (ms *MemoryStorage) GetLastIndex() (uint64, error) { +// LastIndex implements the Storage interface. +func (ms *MemoryStorage) LastIndex() (uint64, error) { ms.Lock() defer ms.Unlock() if len(ms.ents) == 0 { @@ -84,8 +85,8 @@ func (ms *MemoryStorage) GetLastIndex() (uint64, error) { return ms.offset + uint64(len(ms.ents)) - 1, nil } -// GetFirstIndex implements the Storage interface. -func (ms *MemoryStorage) GetFirstIndex() (uint64, error) { +// FirstIndex implements the Storage interface. +func (ms *MemoryStorage) FirstIndex() (uint64, error) { ms.Lock() defer ms.Unlock() return ms.offset, nil