From 7cd824ddceb5bad5ceba2b493b9785cc431fc6d1 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Sun, 28 Jan 2024 00:11:22 +0000 Subject: [PATCH] raft: introduce logSlice struct This is a type-safe wrapper for all kinds of log slices. We will use it for more readable and safe code. Usages will include: wrapping log append requests; unstable struct; possibly surface in a safer API. Signed-off-by: Pavel Kalinnikov --- types.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++- types_test.go | 53 +++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) diff --git a/types.go b/types.go index 8207b213..bb24d434 100644 --- a/types.go +++ b/types.go @@ -14,7 +14,11 @@ package raft -import pb "go.etcd.io/raft/v3/raftpb" +import ( + "fmt" + + pb "go.etcd.io/raft/v3/raftpb" +) // entryID uniquely identifies a raft log entry. // @@ -30,3 +34,73 @@ type entryID struct { func pbEntryID(entry *pb.Entry) entryID { return entryID{term: entry.Term, index: entry.Index} } + +// logSlice describes a correct slice of a raft log. +// +// Every log slice is considered in a context of a specific leader term. This +// term does not necessarily match entryID.term of the entries, since a leader +// log contains both entries from its own term, and some earlier terms. +// +// Two slices with a matching logSlice.term are guaranteed to be consistent, +// i.e. they never contain two different entries at the same index. The reverse +// is not true: two slices with different logSlice.term may contain both +// matching and mismatching entries. Specifically, logs at two different leader +// terms share a common prefix, after which they *permanently* diverge. +// +// A well-formed logSlice conforms to raft safety properties. It provides the +// following guarantees: +// +// 1. entries[i].Index == prev.index + 1 + i, +// 2. prev.term <= entries[0].Term, +// 3. entries[i-1].Term <= entries[i].Term, +// 4. entries[len-1].Term <= term. +// +// Property (1) means the slice is contiguous. Properties (2) and (3) mean that +// the terms of the entries in a log never regress. Property (4) means that a +// leader log at a specific term never has entries from higher terms. +// +// Users of this struct can assume the invariants hold true. Exception is the +// "gateway" code that initially constructs logSlice, such as when its content +// is sourced from a message that was received via transport, or from Storage, +// or in a test code that manually hard-codes this struct. In these cases, the +// invariants should be validated using the valid() method. +type logSlice struct { + // term is the leader term containing the given entries in its log. + term uint64 + // prev is the ID of the entry immediately preceding the entries. + prev entryID + // entries contains the consecutive entries representing this slice. + entries []pb.Entry +} + +// lastIndex returns the index of the last entry in this log slice. Returns +// prev.index if there are no entries. +func (s logSlice) lastIndex() uint64 { + return s.prev.index + uint64(len(s.entries)) +} + +// lastEntryID returns the ID of the last entry in this log slice, or prev if +// there are no entries. +func (s logSlice) lastEntryID() entryID { + if ln := len(s.entries); ln != 0 { + return pbEntryID(&s.entries[ln-1]) + } + return s.prev +} + +// valid returns nil iff the logSlice is a well-formed log slice. See logSlice +// comment for details on what constitutes a valid raft log slice. +func (s logSlice) valid() error { + prev := s.prev + for i := range s.entries { + id := pbEntryID(&s.entries[i]) + if id.term < prev.term || id.index != prev.index+1 { + return fmt.Errorf("leader term %d: entries %+v and %+v not consistent", s.term, prev, id) + } + prev = id + } + if s.term < prev.term { + return fmt.Errorf("leader term %d: entry %+v has a newer term", s.term, prev) + } + return nil +} diff --git a/types_test.go b/types_test.go index 0ac56ada..fd5d91f5 100644 --- a/types_test.go +++ b/types_test.go @@ -39,3 +39,56 @@ func TestEntryID(t *testing.T) { require.Equal(t, tt.want, pbEntryID(&tt.entry)) } } + +func TestLogSlice(t *testing.T) { + id := func(index, term uint64) entryID { + return entryID{term: term, index: index} + } + e := func(index, term uint64) pb.Entry { + return pb.Entry{Term: term, Index: index} + } + for _, tt := range []struct { + term uint64 + prev entryID + entries []pb.Entry + + notOk bool + last entryID + }{ + // Empty "dummy" slice, starting at (0, 0) origin of the log. + {last: id(0, 0)}, + // Empty slice with a given prev ID. Valid only if term >= prev.term. + {prev: id(123, 10), notOk: true}, + {term: 9, prev: id(123, 10), notOk: true}, + {term: 10, prev: id(123, 10), last: id(123, 10)}, + {term: 11, prev: id(123, 10), last: id(123, 10)}, + // A single entry. + {term: 0, entries: []pb.Entry{e(1, 1)}, notOk: true}, + {term: 1, entries: []pb.Entry{e(1, 1)}, last: id(1, 1)}, + {term: 2, entries: []pb.Entry{e(1, 1)}, last: id(1, 1)}, + // Multiple entries. + {term: 2, entries: []pb.Entry{e(2, 1), e(3, 1), e(4, 2)}, notOk: true}, + {term: 1, prev: id(1, 1), entries: []pb.Entry{e(2, 1), e(3, 1), e(4, 2)}, notOk: true}, + {term: 2, prev: id(1, 1), entries: []pb.Entry{e(2, 1), e(3, 1), e(4, 2)}, last: id(4, 2)}, + // First entry inconsistent with prev. + {term: 10, prev: id(123, 5), entries: []pb.Entry{e(111, 5)}, notOk: true}, + {term: 10, prev: id(123, 5), entries: []pb.Entry{e(124, 4)}, notOk: true}, + {term: 10, prev: id(123, 5), entries: []pb.Entry{e(234, 6)}, notOk: true}, + {term: 10, prev: id(123, 5), entries: []pb.Entry{e(124, 6)}, last: id(124, 6)}, + // Inconsistent entries. + {term: 10, prev: id(12, 2), entries: []pb.Entry{e(13, 2), e(12, 2)}, notOk: true}, + {term: 10, prev: id(12, 2), entries: []pb.Entry{e(13, 2), e(15, 2)}, notOk: true}, + {term: 10, prev: id(12, 2), entries: []pb.Entry{e(13, 2), e(14, 1)}, notOk: true}, + {term: 10, prev: id(12, 2), entries: []pb.Entry{e(13, 2), e(14, 3)}, last: id(14, 3)}, + } { + t.Run("", func(t *testing.T) { + s := logSlice{term: tt.term, prev: tt.prev, entries: tt.entries} + require.Equal(t, tt.notOk, s.valid() != nil) + if !tt.notOk { + last := s.lastEntryID() + require.Equal(t, tt.last, last) + require.Equal(t, last.index, s.lastIndex()) + } + }) + } +}