From 90f26e4a560748c9b265fd0e90d68d6834062d91 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 25 Oct 2014 18:49:49 -0700 Subject: [PATCH 1/4] raft: add test for findConflict --- raft/log.go | 10 ++++++++++ raft/log_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/raft/log.go b/raft/log.go index 032fc10d3..eea84a977 100644 --- a/raft/log.go +++ b/raft/log.go @@ -83,6 +83,16 @@ func (l *raftLog) append(after uint64, ents ...pb.Entry) uint64 { return l.lastIndex() } +// 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. +// If there is no conflicting entries, and the existing entries contains +// all the given entries, zero will be returned. +// If there is no conflicting entries, but the given entries contains new +// 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. func (l *raftLog) findConflict(from uint64, ents []pb.Entry) uint64 { for i, ne := range ents { if oe := l.at(from + uint64(i)); oe == nil || oe.Term != ne.Term { diff --git a/raft/log_test.go b/raft/log_test.go index 56ab2b639..dc5e49a0e 100644 --- a/raft/log_test.go +++ b/raft/log_test.go @@ -23,6 +23,42 @@ import ( pb "github.com/coreos/etcd/raft/raftpb" ) +func TestFindConflict(t *testing.T) { + previousEnts := []pb.Entry{{Term: 1}, {Term: 2}, {Term: 3}} + tests := []struct { + from uint64 + ents []pb.Entry + wconflict uint64 + }{ + // no conflict, empty ent + {1, []pb.Entry{}, 0}, + {3, []pb.Entry{}, 0}, + // no conflict + {1, []pb.Entry{{Term: 1}, {Term: 2}, {Term: 3}}, 0}, + {2, []pb.Entry{{Term: 2}, {Term: 3}}, 0}, + {3, []pb.Entry{{Term: 3}}, 0}, + // no conflict, but has new entries + {1, []pb.Entry{{Term: 1}, {Term: 2}, {Term: 3}, {Term: 4}, {Term: 4}}, 4}, + {2, []pb.Entry{{Term: 2}, {Term: 3}, {Term: 4}, {Term: 4}}, 4}, + {3, []pb.Entry{{Term: 3}, {Term: 4}, {Term: 4}}, 4}, + {4, []pb.Entry{{Term: 4}, {Term: 4}}, 4}, + // conflicts with existing entries + {1, []pb.Entry{{Term: 4}, {Term: 4}}, 1}, + {2, []pb.Entry{{Term: 1}, {Term: 4}, {Term: 4}}, 2}, + {3, []pb.Entry{{Term: 1}, {Term: 2}, {Term: 4}, {Term: 4}}, 3}, + } + + for i, tt := range tests { + raftLog := newLog() + raftLog.ents = append(raftLog.ents, previousEnts...) + + gconflict := raftLog.findConflict(tt.from, tt.ents) + if gconflict != tt.wconflict { + t.Errorf("#%d: conflict = %d, want %d", i, gconflict, tt.wconflict) + } + } +} + // TestAppend ensures: // 1. If an existing entry conflicts with a new one (same index // but different terms), delete the existing entry and all that From 8cd95e916da9894cfc80c674ce0835855eb5fef8 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 25 Oct 2014 20:12:54 -0700 Subject: [PATCH 2/4] raft: comments for isUpToDate --- raft/log.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/raft/log.go b/raft/log.go index eea84a977..589519668 100644 --- a/raft/log.go +++ b/raft/log.go @@ -152,9 +152,14 @@ func (l *raftLog) entries(i uint64) []pb.Entry { return l.slice(i, l.lastIndex()+1) } -func (l *raftLog) isUpToDate(i, term uint64) bool { +// isUpToDate determines if the given (lastIndex,term) log is more up-to-date +// 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 +// 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. +func (l *raftLog) isUpToDate(lasti, term uint64) bool { e := l.at(l.lastIndex()) - return term > e.Term || (term == e.Term && i >= l.lastIndex()) + return term > e.Term || (term == e.Term && lasti >= l.lastIndex()) } func (l *raftLog) matchTerm(i, term uint64) bool { From 94f701cf95803373fba6f26ed28a3a1397505d36 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 25 Oct 2014 20:34:14 -0700 Subject: [PATCH 3/4] raft: refactor isUpToDate and add a test --- raft/log.go | 9 ++++----- raft/log_test.go | 31 +++++++++++++++++++++++++++++++ raft/raft.go | 3 +-- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/raft/log.go b/raft/log.go index 589519668..57e902cca 100644 --- a/raft/log.go +++ b/raft/log.go @@ -131,9 +131,9 @@ 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) term(i uint64) uint64 { if e := l.at(i); e != nil { @@ -158,8 +158,7 @@ func (l *raftLog) entries(i uint64) []pb.Entry { // 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. func (l *raftLog) isUpToDate(lasti, term uint64) bool { - e := l.at(l.lastIndex()) - return term > e.Term || (term == e.Term && lasti >= l.lastIndex()) + return term > l.lastTerm() || (term == l.lastTerm() && lasti >= l.lastIndex()) } func (l *raftLog) matchTerm(i, term uint64) bool { diff --git a/raft/log_test.go b/raft/log_test.go index dc5e49a0e..a555acda9 100644 --- a/raft/log_test.go +++ b/raft/log_test.go @@ -59,6 +59,37 @@ func TestFindConflict(t *testing.T) { } } +func TestIsUpToDate(t *testing.T) { + previousEnts := []pb.Entry{{Term: 1}, {Term: 2}, {Term: 3}} + raftLog := newLog() + raftLog.ents = append(raftLog.ents, previousEnts...) + tests := []struct { + lastIndex uint64 + term uint64 + wUpToDate bool + }{ + // greater term, ignore lastIndex + {raftLog.lastIndex() - 1, 4, true}, + {raftLog.lastIndex(), 4, true}, + {raftLog.lastIndex() + 1, 4, true}, + // smaller term, ignore lastIndex + {raftLog.lastIndex() - 1, 2, false}, + {raftLog.lastIndex(), 2, false}, + {raftLog.lastIndex() + 1, 2, false}, + // equal term, lager lastIndex wins + {raftLog.lastIndex() - 1, 3, false}, + {raftLog.lastIndex(), 3, true}, + {raftLog.lastIndex() + 1, 3, true}, + } + + for i, tt := range tests { + gUpToDate := raftLog.isUpToDate(tt.lastIndex, tt.term) + if gUpToDate != tt.wUpToDate { + t.Errorf("#%d: uptodate = %v, want %v", i, gUpToDate, tt.wUpToDate) + } + } +} + // TestAppend ensures: // 1. If an existing entry conflicts with a new one (same index // but different terms), delete the existing entry and all that diff --git a/raft/raft.go b/raft/raft.go index b2e7e8f9c..230c32e1e 100644 --- a/raft/raft.go +++ b/raft/raft.go @@ -348,8 +348,7 @@ func (r *raft) campaign() { if i == r.id { continue } - lasti := r.raftLog.lastIndex() - r.send(pb.Message{To: i, Type: pb.MsgVote, Index: lasti, LogTerm: r.raftLog.term(lasti)}) + r.send(pb.Message{To: i, Type: pb.MsgVote, Index: r.raftLog.lastIndex(), LogTerm: r.raftLog.lastTerm()}) } } From 460d6490ba491e8cfe3c6320f46ccfc7e48a4757 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 27 Oct 2014 11:09:44 -0700 Subject: [PATCH 4/4] raft: address issues in comments --- raft/log.go | 17 ++++++++++------- raft/log_test.go | 8 ++++---- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/raft/log.go b/raft/log.go index 57e902cca..2e853a23a 100644 --- a/raft/log.go +++ b/raft/log.go @@ -84,16 +84,18 @@ func (l *raftLog) append(after uint64, ents ...pb.Entry) uint64 { } // 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 // all the given entries, zero will be returned. // If there is no conflicting entries, but the given entries contains new // 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 { + // TODO(xiangli): validate the index of ents for i, ne := range ents { if oe := l.at(from + uint64(i)); oe == nil || oe.Term != ne.Term { 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) lastTerm() uint64 { return l.term(l.lastIndex()) } +func (l *raftLog) lastTerm() uint64 { return l.term(l.lastIndex()) } func (l *raftLog) term(i uint64) uint64 { 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. // 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 -// 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 { return term > l.lastTerm() || (term == l.lastTerm() && lasti >= l.lastIndex()) } diff --git a/raft/log_test.go b/raft/log_test.go index a555acda9..3cfa41a31 100644 --- a/raft/log_test.go +++ b/raft/log_test.go @@ -50,7 +50,7 @@ func TestFindConflict(t *testing.T) { for i, tt := range tests { raftLog := newLog() - raftLog.ents = append(raftLog.ents, previousEnts...) + raftLog.append(raftLog.lastIndex(), previousEnts...) gconflict := raftLog.findConflict(tt.from, tt.ents) if gconflict != tt.wconflict { @@ -62,7 +62,7 @@ func TestFindConflict(t *testing.T) { func TestIsUpToDate(t *testing.T) { previousEnts := []pb.Entry{{Term: 1}, {Term: 2}, {Term: 3}} raftLog := newLog() - raftLog.ents = append(raftLog.ents, previousEnts...) + raftLog.append(raftLog.lastIndex(), previousEnts...) tests := []struct { lastIndex uint64 term uint64 @@ -139,7 +139,7 @@ func TestAppend(t *testing.T) { for i, tt := range tests { raftLog := newLog() - raftLog.ents = append(raftLog.ents, previousEnts...) + raftLog.append(raftLog.lastIndex(), previousEnts...) raftLog.unstable = previousUnstable index := raftLog.append(tt.after, tt.ents...) if index != tt.windex { @@ -219,7 +219,7 @@ func TestUnstableEnts(t *testing.T) { for i, tt := range tests { raftLog := newLog() - raftLog.ents = append(raftLog.ents, previousEnts...) + raftLog.append(0, previousEnts...) raftLog.unstable = tt.unstable ents := raftLog.unstableEnts() raftLog.resetUnstable()