From 96e0a0535c8f1aa82f26e5e6542146c05544dafe Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 1 Feb 2024 22:34:36 +0000 Subject: [PATCH 1/3] log: consolidate raftLog initialization Signed-off-by: Pavel Kalinnikov --- log.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/log.go b/log.go index 9097491c..4f96662d 100644 --- a/log.go +++ b/log.go @@ -75,11 +75,6 @@ func newLogWithSize(storage Storage, logger Logger, maxApplyingEntsSize entryEnc if storage == nil { log.Panic("storage must not be nil") } - log := &raftLog{ - storage: storage, - logger: logger, - maxApplyingEntsSize: maxApplyingEntsSize, - } firstIndex, err := storage.FirstIndex() if err != nil { panic(err) // TODO(bdarnell) @@ -88,15 +83,22 @@ func newLogWithSize(storage Storage, logger Logger, maxApplyingEntsSize entryEnc if err != nil { panic(err) // TODO(bdarnell) } - log.unstable.offset = lastIndex + 1 - log.unstable.offsetInProgress = lastIndex + 1 - log.unstable.logger = logger - // Initialize our committed and applied pointers to the time of the last compaction. - log.committed = firstIndex - 1 - log.applying = firstIndex - 1 - log.applied = firstIndex - 1 + return &raftLog{ + storage: storage, + unstable: unstable{ + offset: lastIndex + 1, + offsetInProgress: lastIndex + 1, + logger: logger, + }, + maxApplyingEntsSize: maxApplyingEntsSize, - return log + // Initialize our committed and applied pointers to the time of the last compaction. + committed: firstIndex - 1, + applying: firstIndex - 1, + applied: firstIndex - 1, + + logger: logger, + } } func (l *raftLog) String() string { From 83236d4267aee93bc01d3b1f5a955c9a9dae2a87 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 1 Feb 2024 22:48:02 +0000 Subject: [PATCH 2/3] tests: init raftLog using the newLog function Signed-off-by: Pavel Kalinnikov --- raft_test.go | 47 ++++++++++++++--------------------------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/raft_test.go b/raft_test.go index a10f40be..5a258e56 100644 --- a/raft_test.go +++ b/raft_test.go @@ -1002,13 +1002,10 @@ func TestCandidateConcede(t *testing.T) { if g := a.Term; g != 1 { t.Errorf("term = %d, want %d", g, 1) } - wantLog := ltoa(&raftLog{ - storage: &MemoryStorage{ - ents: []pb.Entry{{}, {Data: nil, Term: 1, Index: 1}, {Term: 1, Index: 2, Data: data}}, - }, - unstable: unstable{offset: 3}, - committed: 2, - }) + + wantLog := ltoa(newLog(&MemoryStorage{ + ents: []pb.Entry{{}, {Data: nil, Term: 1, Index: 1}, {Term: 1, Index: 2, Data: data}}, + }, nil)) for i, p := range tt.peers { if sm, ok := p.(*raft); ok { l := ltoa(sm.raftLog) @@ -1054,11 +1051,7 @@ func TestOldMessages(t *testing.T) { ents := index(0).terms(0, 1, 2, 3, 3) ents[4].Data = []byte("somedata") - ilog := &raftLog{ - storage: &MemoryStorage{ents: ents}, - unstable: unstable{offset: 5}, - committed: 4, - } + ilog := newLog(&MemoryStorage{ents: ents}, nil) base := ltoa(ilog) for i, p := range tt.peers { if sm, ok := p.(*raft); ok { @@ -1109,12 +1102,9 @@ func TestProposal(t *testing.T) { wantLog := newLog(NewMemoryStorage(), raftLogger) if tt.success { - wantLog = &raftLog{ - storage: &MemoryStorage{ - ents: []pb.Entry{{}, {Data: nil, Term: 1, Index: 1}, {Term: 1, Index: 2, Data: data}}, - }, - unstable: unstable{offset: 3}, - } + wantLog = newLog(&MemoryStorage{ + ents: []pb.Entry{{}, {Data: nil, Term: 1, Index: 1}, {Term: 1, Index: 2, Data: data}}, + }, nil) } base := ltoa(wantLog) for i, p := range tt.peers { @@ -1147,12 +1137,9 @@ func TestProposalByProxy(t *testing.T) { // propose via follower tt.send(pb.Message{From: 2, To: 2, Type: pb.MsgProp, Entries: []pb.Entry{{Data: []byte("somedata")}}}) - wantLog := &raftLog{ - storage: &MemoryStorage{ - ents: []pb.Entry{{}, {Data: nil, Term: 1, Index: 1}, {Term: 1, Data: data, Index: 2}}, - }, - unstable: unstable{offset: 3}, - committed: 2} + wantLog := newLog(&MemoryStorage{ + ents: []pb.Entry{{}, {Data: nil, Term: 1, Index: 1}, {Term: 1, Data: data, Index: 2}}, + }, nil) base := ltoa(wantLog) for i, p := range tt.peers { if sm, ok := p.(*raft); ok { @@ -1566,10 +1553,7 @@ func testRecvMsgVote(t *testing.T, msgType pb.MessageType) { sm.step = stepLeader } sm.Vote = tt.voteFor - sm.raftLog = &raftLog{ - storage: &MemoryStorage{ents: index(0).terms(0, 2, 2)}, - unstable: unstable{offset: 3}, - } + sm.raftLog = newLog(&MemoryStorage{ents: index(0).terms(0, 2, 2)}, nil) // raft.Term is greater than or equal to raft.raftLog.lastTerm. In this // test we're only testing MsgVote responses when the campaigning node @@ -2606,10 +2590,7 @@ func TestLeaderAppResp(t *testing.T) { // sm term is 1 after it becomes the leader. // thus the last log term must be 1 to be committed. sm := newTestRaft(1, 10, 1, newTestMemoryStorage(withPeers(1, 2, 3))) - sm.raftLog = &raftLog{ - storage: &MemoryStorage{ents: index(0).terms(0, 1, 1)}, - unstable: unstable{offset: 3}, - } + sm.raftLog = newLog(&MemoryStorage{ents: index(0).terms(0, 1, 1)}, nil) sm.becomeCandidate() sm.becomeLeader() sm.readMessages() @@ -2721,7 +2702,7 @@ func TestRecvMsgBeat(t *testing.T) { for i, tt := range tests { sm := newTestRaft(1, 10, 1, newTestMemoryStorage(withPeers(1, 2, 3))) - sm.raftLog = &raftLog{storage: &MemoryStorage{ents: index(0).terms(0, 1, 1)}} + sm.raftLog = newLog(&MemoryStorage{ents: index(0).terms(0, 1, 1)}, nil) sm.Term = 1 sm.state = tt.state switch tt.state { From 38da3a3dae3e3940375858524bfb7fc91ea985ec Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 1 Feb 2024 23:05:14 +0000 Subject: [PATCH 3/3] log: remove the unnecessary nil check Signed-off-by: Pavel Kalinnikov --- log.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/log.go b/log.go index 4f96662d..bd7c2feb 100644 --- a/log.go +++ b/log.go @@ -16,7 +16,6 @@ package raft import ( "fmt" - "log" pb "go.etcd.io/raft/v3/raftpb" ) @@ -72,9 +71,6 @@ func newLog(storage Storage, logger Logger) *raftLog { // newLogWithSize returns a log using the given storage and max // message size. func newLogWithSize(storage Storage, logger Logger, maxApplyingEntsSize entryEncodingSize) *raftLog { - if storage == nil { - log.Panic("storage must not be nil") - } firstIndex, err := storage.FirstIndex() if err != nil { panic(err) // TODO(bdarnell)