Browse Source

raft: address issues in comments

Xiang Li 11 years ago
parent
commit
460d6490ba
2 changed files with 14 additions and 11 deletions
  1. 10 7
      raft/log.go
  2. 4 4
      raft/log_test.go

+ 10 - 7
raft/log.go

@@ -84,16 +84,18 @@ func (l *raftLog) append(after uint64, ents ...pb.Entry) uint64 {
 }
 }
 
 
 // findConflict finds the index of the conflict.
 // findConflict finds the index of the conflict.
-// It returns the the first pair of confilcting entries between the existing
-// entries and the given entries, if there is any.
+// It returns the first pair of conflicting entries between the existing
+// entries and the given entries, if there are any.
 // If there is no conflicting entries, and the existing entries contains
 // If there is no conflicting entries, and the existing entries contains
 // all the given entries, zero will be returned.
 // all the given entries, zero will be returned.
 // If there is no conflicting entries, but the given entries contains new
 // If there is no conflicting entries, but the given entries contains new
 // entries, the index of the first new entry will be returned.
 // entries, the index of the first new entry will be returned.
-// Conflicting entries has the same index but different term.
-// The first given entry MUST have the index equal to from.
-// The index of the given entries MUST be continously increasing.
+// An entry is considered to be conflicting if it has the same index but
+// a different term.
+// The first entry MUST have an index equal to the argument 'from'.
+// The index of the given entries MUST be continuously increasing.
 func (l *raftLog) findConflict(from uint64, ents []pb.Entry) uint64 {
 func (l *raftLog) findConflict(from uint64, ents []pb.Entry) uint64 {
+	// TODO(xiangli): validate the index of ents
 	for i, ne := range ents {
 	for i, ne := range ents {
 		if oe := l.at(from + uint64(i)); oe == nil || oe.Term != ne.Term {
 		if oe := l.at(from + uint64(i)); oe == nil || oe.Term != ne.Term {
 			return from + uint64(i)
 			return from + uint64(i)
@@ -133,7 +135,7 @@ func (l *raftLog) resetNextEnts() {
 
 
 func (l *raftLog) lastIndex() uint64 { return uint64(len(l.ents)) - 1 + l.offset }
 func (l *raftLog) lastIndex() uint64 { return uint64(len(l.ents)) - 1 + l.offset }
 
 
-func (l raftLog) lastTerm() uint64 { return l.term(l.lastIndex()) }
+func (l *raftLog) lastTerm() uint64 { return l.term(l.lastIndex()) }
 
 
 func (l *raftLog) term(i uint64) uint64 {
 func (l *raftLog) term(i uint64) uint64 {
 	if e := l.at(i); e != nil {
 	if e := l.at(i); e != nil {
@@ -156,7 +158,8 @@ func (l *raftLog) entries(i uint64) []pb.Entry {
 // by comparing the index and term of the last entries in the existing logs.
 // by comparing the index and term of the last entries in the existing logs.
 // If the logs have last entries with different terms, then the log with the
 // If the logs have last entries with different terms, then the log with the
 // later term is more up-to-date. If the logs end with the same term, then
 // later term is more up-to-date. If the logs end with the same term, then
-// whichever log has the larger lastIndex is more up-to-date.
+// whichever log has the larger lastIndex is more up-to-date. If the logs are
+// the same, the given log is up-to-date.
 func (l *raftLog) isUpToDate(lasti, term uint64) bool {
 func (l *raftLog) isUpToDate(lasti, term uint64) bool {
 	return term > l.lastTerm() || (term == l.lastTerm() && lasti >= l.lastIndex())
 	return term > l.lastTerm() || (term == l.lastTerm() && lasti >= l.lastIndex())
 }
 }

+ 4 - 4
raft/log_test.go

@@ -50,7 +50,7 @@ func TestFindConflict(t *testing.T) {
 
 
 	for i, tt := range tests {
 	for i, tt := range tests {
 		raftLog := newLog()
 		raftLog := newLog()
-		raftLog.ents = append(raftLog.ents, previousEnts...)
+		raftLog.append(raftLog.lastIndex(), previousEnts...)
 
 
 		gconflict := raftLog.findConflict(tt.from, tt.ents)
 		gconflict := raftLog.findConflict(tt.from, tt.ents)
 		if gconflict != tt.wconflict {
 		if gconflict != tt.wconflict {
@@ -62,7 +62,7 @@ func TestFindConflict(t *testing.T) {
 func TestIsUpToDate(t *testing.T) {
 func TestIsUpToDate(t *testing.T) {
 	previousEnts := []pb.Entry{{Term: 1}, {Term: 2}, {Term: 3}}
 	previousEnts := []pb.Entry{{Term: 1}, {Term: 2}, {Term: 3}}
 	raftLog := newLog()
 	raftLog := newLog()
-	raftLog.ents = append(raftLog.ents, previousEnts...)
+	raftLog.append(raftLog.lastIndex(), previousEnts...)
 	tests := []struct {
 	tests := []struct {
 		lastIndex uint64
 		lastIndex uint64
 		term      uint64
 		term      uint64
@@ -139,7 +139,7 @@ func TestAppend(t *testing.T) {
 
 
 	for i, tt := range tests {
 	for i, tt := range tests {
 		raftLog := newLog()
 		raftLog := newLog()
-		raftLog.ents = append(raftLog.ents, previousEnts...)
+		raftLog.append(raftLog.lastIndex(), previousEnts...)
 		raftLog.unstable = previousUnstable
 		raftLog.unstable = previousUnstable
 		index := raftLog.append(tt.after, tt.ents...)
 		index := raftLog.append(tt.after, tt.ents...)
 		if index != tt.windex {
 		if index != tt.windex {
@@ -219,7 +219,7 @@ func TestUnstableEnts(t *testing.T) {
 
 
 	for i, tt := range tests {
 	for i, tt := range tests {
 		raftLog := newLog()
 		raftLog := newLog()
-		raftLog.ents = append(raftLog.ents, previousEnts...)
+		raftLog.append(0, previousEnts...)
 		raftLog.unstable = tt.unstable
 		raftLog.unstable = tt.unstable
 		ents := raftLog.unstableEnts()
 		ents := raftLog.unstableEnts()
 		raftLog.resetUnstable()
 		raftLog.resetUnstable()