Browse Source

raft: PR feedback.

Removed Get prefix in method names, added assertions and fixed comments.
Ben Darnell 11 years ago
parent
commit
45e96be605
3 changed files with 29 additions and 23 deletions
  1. 14 9
      raft/log.go
  2. 2 2
      raft/log_test.go
  3. 13 12
      raft/storage.go

+ 14 - 9
raft/log.go

@@ -52,7 +52,7 @@ func newLog(storage Storage) *raftLog {
 	log := &raftLog{
 	log := &raftLog{
 		storage: storage,
 		storage: storage,
 	}
 	}
-	lastIndex, err := storage.GetLastIndex()
+	lastIndex, err := storage.LastIndex()
 	if err == ErrStorageEmpty {
 	if err == ErrStorageEmpty {
 		// When starting from scratch populate the list with a dummy entry at term zero.
 		// When starting from scratch populate the list with a dummy entry at term zero.
 		log.unstableEnts = make([]pb.Entry, 1)
 		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 {
 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 {
 	if after < l.unstable {
 		// The log is being truncated to before our current unstable
 		// The log is being truncated to before our current unstable
 		// portion, so discard it and reset unstable.
 		// portion, so discard it and reset unstable.
@@ -132,9 +135,7 @@ func (l *raftLog) unstableEntries() []pb.Entry {
 	if len(l.unstableEnts) == 0 {
 	if len(l.unstableEnts) == 0 {
 		return nil
 		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.
 // nextEnts returns all the available entries for execution.
@@ -147,7 +148,7 @@ func (l *raftLog) nextEnts() (ents []pb.Entry) {
 }
 }
 
 
 func (l *raftLog) firstIndex() uint64 {
 func (l *raftLog) firstIndex() uint64 {
-	index, err := l.storage.GetFirstIndex()
+	index, err := l.storage.FirstIndex()
 	if err != nil {
 	if err != nil {
 		panic(err) // TODO(bdarnell)
 		panic(err) // TODO(bdarnell)
 	}
 	}
@@ -158,7 +159,7 @@ func (l *raftLog) lastIndex() uint64 {
 	if len(l.unstableEnts) > 0 {
 	if len(l.unstableEnts) > 0 {
 		return l.unstable + uint64(len(l.unstableEnts)) - 1
 		return l.unstable + uint64(len(l.unstableEnts)) - 1
 	}
 	}
-	index, err := l.storage.GetLastIndex()
+	index, err := l.storage.LastIndex()
 	if err != nil {
 	if err != nil {
 		panic(err) // TODO(bdarnell)
 		panic(err) // TODO(bdarnell)
 	}
 	}
@@ -176,6 +177,10 @@ func (l *raftLog) appliedTo(i uint64) {
 }
 }
 
 
 func (l *raftLog) stableTo(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.unstableEnts = l.unstableEnts[i+1-l.unstable:]
 	l.unstable = i + 1
 	l.unstable = i + 1
 }
 }
@@ -247,11 +252,11 @@ func (l *raftLog) compact(i uint64) uint64 {
 		panic(err) // TODO(bdarnell)
 		panic(err) // TODO(bdarnell)
 	}
 	}
 	l.unstable = max(i+1, l.unstable)
 	l.unstable = max(i+1, l.unstable)
-	firstIndex, err := l.storage.GetFirstIndex()
+	firstIndex, err := l.storage.FirstIndex()
 	if err != nil {
 	if err != nil {
 		panic(err) // TODO(bdarnell)
 		panic(err) // TODO(bdarnell)
 	}
 	}
-	lastIndex, err := l.storage.GetLastIndex()
+	lastIndex, err := l.storage.LastIndex()
 	if err != nil {
 	if err != nil {
 		panic(err) // TODO(bdarnell)
 		panic(err) // TODO(bdarnell)
 	}
 	}
@@ -297,7 +302,7 @@ func (l *raftLog) slice(lo uint64, hi uint64) []pb.Entry {
 	}
 	}
 	var ents []pb.Entry
 	var ents []pb.Entry
 	if lo < l.unstable {
 	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 {
 		if err != nil {
 			panic(err) // TODO(bdarnell)
 			panic(err) // TODO(bdarnell)
 		}
 		}

+ 2 - 2
raft/log_test.go

@@ -495,7 +495,7 @@ func TestAt(t *testing.T) {
 
 
 	l := newLog(NewMemoryStorage())
 	l := newLog(NewMemoryStorage())
 	l.restore(pb.Snapshot{Index: offset})
 	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})
 		l.append(offset+i-1, pb.Entry{Term: i})
 	}
 	}
 
 
@@ -525,7 +525,7 @@ func TestSlice(t *testing.T) {
 
 
 	l := newLog(NewMemoryStorage())
 	l := newLog(NewMemoryStorage())
 	l.restore(pb.Snapshot{Index: offset})
 	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})
 		l.append(offset+i-1, pb.Entry{Term: i})
 	}
 	}
 
 

+ 13 - 12
raft/storage.go

@@ -34,17 +34,18 @@ var ErrStorageEmpty = errors.New("storage is empty")
 // become inoperable and refuse to participate in elections; the
 // become inoperable and refuse to participate in elections; the
 // application is responsible for cleanup and recovery in this case.
 // application is responsible for cleanup and recovery in this case.
 type Storage interface {
 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.
 	// GetLastIndex returns the index of the last entry in the log.
 	// If the log is empty it returns ErrStorageEmpty.
 	// 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
 	// GetFirstIndex returns the index of the first log entry that is
 	// available via GetEntries (older entries have been incorporated
 	// available via GetEntries (older entries have been incorporated
 	// into the latest Snapshot).
 	// 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
 	Compact(i uint64) error
 }
 }
 
 
@@ -67,15 +68,15 @@ func NewMemoryStorage() *MemoryStorage {
 	return &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()
 	ms.Lock()
 	defer ms.Unlock()
 	defer ms.Unlock()
 	return ms.ents[lo-ms.offset : hi-ms.offset], nil
 	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()
 	ms.Lock()
 	defer ms.Unlock()
 	defer ms.Unlock()
 	if len(ms.ents) == 0 {
 	if len(ms.ents) == 0 {
@@ -84,8 +85,8 @@ func (ms *MemoryStorage) GetLastIndex() (uint64, error) {
 	return ms.offset + uint64(len(ms.ents)) - 1, nil
 	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()
 	ms.Lock()
 	defer ms.Unlock()
 	defer ms.Unlock()
 	return ms.offset, nil
 	return ms.offset, nil