Browse Source

raft: replace StatusWithoutProgress with BasicStatus

Now that a Config is also added to the full status, the old name
did not convey the intention, which was to get a Status without
an associated allocation.
Tobias Schottdorf 6 years ago
parent
commit
6b0322549f
3 changed files with 27 additions and 22 deletions
  1. 8 9
      raft/rawnode.go
  2. 4 6
      raft/rawnode_test.go
  3. 15 7
      raft/status.go

+ 8 - 9
raft/rawnode.go

@@ -218,18 +218,17 @@ func (rn *RawNode) Advance(rd Ready) {
 	rn.commitReady(rd)
 }
 
-// Status returns the current status of the given group.
-func (rn *RawNode) Status() *Status {
+// Status returns the current status of the given group. This allocates, see
+// BasicStatus and WithProgress for allocation-friendlier choices.
+func (rn *RawNode) Status() Status {
 	status := getStatus(rn.raft)
-	return &status
+	return status
 }
 
-// StatusWithoutProgress returns a Status without populating the Progress field
-// (and returns the Status as a value to avoid forcing it onto the heap). This
-// is more performant if the Progress is not required. See WithProgress for an
-// allocation-free way to introspect the Progress.
-func (rn *RawNode) StatusWithoutProgress() Status {
-	return getStatusWithoutProgress(rn.raft)
+// BasicStatus returns a BasicStatus. Notably this does not contain the
+// Progress map; see WithProgress for an allocation-free way to inspect it.
+func (rn *RawNode) BasicStatus() BasicStatus {
+	return getBasicStatus(rn.raft)
 }
 
 // ProgressType indicates the type of replica a Progress corresponds to.

+ 4 - 6
raft/rawnode_test.go

@@ -44,7 +44,7 @@ func (a *rawNodeAdapter) TransferLeadership(ctx context.Context, lead, transfere
 func (a *rawNodeAdapter) Stop() {}
 
 // RawNode returns a *Status.
-func (a *rawNodeAdapter) Status() Status { return *a.RawNode.Status() }
+func (a *rawNodeAdapter) Status() Status { return a.RawNode.Status() }
 
 // RawNode takes a Ready. It doesn't really have to do that I think? It can hold on
 // to it internally. But maybe that approach is frail.
@@ -610,7 +610,7 @@ func TestRawNodeBoundedLogGrowthWithPartition(t *testing.T) {
 	checkUncommitted(0)
 }
 
-func BenchmarkStatusProgress(b *testing.B) {
+func BenchmarkStatus(b *testing.B) {
 	setup := func(members int) *RawNode {
 		peers := make([]uint64, members)
 		for i := range peers {
@@ -627,8 +627,6 @@ func BenchmarkStatusProgress(b *testing.B) {
 
 	for _, members := range []int{1, 3, 5, 100} {
 		b.Run(fmt.Sprintf("members=%d", members), func(b *testing.B) {
-			// NB: call getStatus through rn.Status because that incurs an additional
-			// allocation.
 			rn := setup(members)
 
 			b.Run("Status", func(b *testing.B) {
@@ -650,10 +648,10 @@ func BenchmarkStatusProgress(b *testing.B) {
 				}
 			})
 
-			b.Run("StatusWithoutProgress", func(b *testing.B) {
+			b.Run("BasicStatus", func(b *testing.B) {
 				b.ReportAllocs()
 				for i := 0; i < b.N; i++ {
-					_ = rn.StatusWithoutProgress()
+					_ = rn.BasicStatus()
 				}
 			})
 

+ 15 - 7
raft/status.go

@@ -21,15 +21,22 @@ import (
 	"go.etcd.io/etcd/raft/tracker"
 )
 
+// Status contains information about this Raft peer and its view of the system.
+// The Progress is only populated on the leader.
 type Status struct {
+	BasicStatus
+	Config   tracker.Config
+	Progress map[uint64]tracker.Progress
+}
+
+// BasicStatus contains basic information about the Raft peer. It does not allocate.
+type BasicStatus struct {
 	ID uint64
 
 	pb.HardState
 	SoftState
 
-	Applied  uint64
-	Config   tracker.Config
-	Progress map[uint64]tracker.Progress
+	Applied uint64
 
 	LeadTransferee uint64
 }
@@ -47,24 +54,25 @@ func getProgressCopy(r *raft) map[uint64]tracker.Progress {
 	return m
 }
 
-func getStatusWithoutProgress(r *raft) Status {
-	s := Status{
+func getBasicStatus(r *raft) BasicStatus {
+	s := BasicStatus{
 		ID:             r.id,
 		LeadTransferee: r.leadTransferee,
 	}
 	s.HardState = r.hardState()
 	s.SoftState = *r.softState()
 	s.Applied = r.raftLog.applied
-	s.Config = r.prs.Config.Clone()
 	return s
 }
 
 // getStatus gets a copy of the current raft status.
 func getStatus(r *raft) Status {
-	s := getStatusWithoutProgress(r)
+	var s Status
+	s.BasicStatus = getBasicStatus(r)
 	if s.RaftState == StateLeader {
 		s.Progress = getProgressCopy(r)
 	}
+	s.Config = r.prs.Config.Clone()
 	return s
 }