Merge pull request #14592 from nvanbenschoten/nvanbenschoten/nilSnapMsg

raft: make Message.Snapshot nullable, halve struct size

contrib/raftexample/raft_test.go

@@ -37,7 +37,7 @@ func TestProcessMessages(t *testing.T) {
 				{
 					Type: raftpb.MsgSnap,
 					To:   8,
-					Snapshot: raftpb.Snapshot{
+					Snapshot: &raftpb.Snapshot{
 						Metadata: raftpb.SnapshotMetadata{
 							Index: 100,
 							Term:  3,
@@ -53,7 +53,7 @@ func TestProcessMessages(t *testing.T) {
 				{
 					Type: raftpb.MsgSnap,
 					To:   8,
-					Snapshot: raftpb.Snapshot{
+					Snapshot: &raftpb.Snapshot{
 						Metadata: raftpb.SnapshotMetadata{
 							Index: 100,
 							Term:  3,
@@ -74,7 +74,7 @@ func TestProcessMessages(t *testing.T) {
 				{
 					Type: raftpb.MsgSnap,
 					To:   8,
-					Snapshot: raftpb.Snapshot{
+					Snapshot: &raftpb.Snapshot{
 						Metadata: raftpb.SnapshotMetadata{
 							Index: 100,
 							Term:  3,
@@ -95,7 +95,7 @@ func TestProcessMessages(t *testing.T) {
 				{
 					Type: raftpb.MsgSnap,
 					To:   8,
-					Snapshot: raftpb.Snapshot{
+					Snapshot: &raftpb.Snapshot{
 						Metadata: raftpb.SnapshotMetadata{
 							Index: 100,
 							Term:  3,

raft/raft.go

@@ -478,7 +478,7 @@ func (r *raft) maybeSendAppend(to uint64, sendIfEmpty bool) bool {
 		pr.BecomeSnapshot(sindex)
 		r.logger.Debugf("%x paused sending replication messages to %x [%s]", r.id, to, pr)
 
-		r.send(pb.Message{To: to, Type: pb.MsgSnap, Snapshot: snapshot})
+		r.send(pb.Message{To: to, Type: pb.MsgSnap, Snapshot: &snapshot})
 		return true
 	}
 
@@ -1537,8 +1537,14 @@ func (r *raft) handleHeartbeat(m pb.Message) {
 }
 
 func (r *raft) handleSnapshot(m pb.Message) {
-	sindex, sterm := m.Snapshot.Metadata.Index, m.Snapshot.Metadata.Term
-	if r.restore(m.Snapshot) {
+	// MsgSnap messages should always carry a non-nil Snapshot, but err on the
+	// side of safety and treat a nil Snapshot as a zero-valued Snapshot.
+	var s pb.Snapshot
+	if m.Snapshot != nil {
+		s = *m.Snapshot
+	}
+	sindex, sterm := s.Metadata.Index, s.Metadata.Term
+	if r.restore(s) {
 		r.logger.Infof("%x [commit: %d] restored snapshot [index: %d, term: %d]",
 			r.id, r.raftLog.committed, sindex, sterm)
 		r.send(pb.Message{To: m.From, Type: pb.MsgAppResp, Index: r.raftLog.lastIndex()})

raft/raft_test.go

@@ -3067,7 +3067,7 @@ func TestIgnoreProvidingSnap(t *testing.T) {
 }
 
 func TestRestoreFromSnapMsg(t *testing.T) {
-	s := pb.Snapshot{
+	s := &pb.Snapshot{
 		Metadata: pb.SnapshotMetadata{
 			Index:     11, // magic number
 			Term:      11, // magic number

raft/raftpb/raft.proto

@@ -76,7 +76,11 @@ message Message {
 	optional uint64      index       = 6  [(gogoproto.nullable) = false];
 	repeated Entry       entries     = 7  [(gogoproto.nullable) = false];
 	optional uint64      commit      = 8  [(gogoproto.nullable) = false];
-	optional Snapshot    snapshot    = 9  [(gogoproto.nullable) = false];
+	// snapshot is non-nil and non-empty for MsgSnap messages and nil for all other
+	// message types. However, peer nodes running older binary versions may send a
+	// non-nil, empty value for the snapshot field of non-MsgSnap messages. Code
+	// should be prepared to handle such messages.
+	optional Snapshot    snapshot    = 9  [(gogoproto.nullable) = true];
 	optional bool        reject      = 10 [(gogoproto.nullable) = false];
 	optional uint64      rejectHint  = 11 [(gogoproto.nullable) = false];
 	optional bytes       context     = 12;

raft/raftpb/raft_test.go

@@ -45,7 +45,7 @@ func TestProtoMemorySizes(t *testing.T) {
 	assert(unsafe.Sizeof(s), if64Bit(144, 80), "Snapshot")
 
 	var m Message
-	assert(unsafe.Sizeof(m), if64Bit(264, 168), "Message")
+	assert(unsafe.Sizeof(m), if64Bit(128, 92), "Message")
 
 	var hs HardState
 	assert(unsafe.Sizeof(hs), 24, "HardState")

raft/util.go

@@ -149,8 +149,8 @@ func DescribeMessage(m pb.Message, f EntryFormatter) string {
 		}
 		fmt.Fprintf(&buf, "]")
 	}
-	if !IsEmptySnap(m.Snapshot) {
-		fmt.Fprintf(&buf, " Snapshot: %s", DescribeSnapshot(m.Snapshot))
+	if s := m.Snapshot; s != nil && !IsEmptySnap(*s) {
+		fmt.Fprintf(&buf, " Snapshot: %s", DescribeSnapshot(*s))
 	}
 	return buf.String()
 }

server/etcdserver/api/rafthttp/functional_test.go

@@ -71,7 +71,7 @@ func TestSendMessage(t *testing.T) {
 		{Type: raftpb.MsgAppResp, From: 1, To: 2, Term: 1, Index: 3},
 		{Type: raftpb.MsgVote, From: 1, To: 2, Term: 1, Index: 3, LogTerm: 0},
 		{Type: raftpb.MsgVoteResp, From: 1, To: 2, Term: 1},
-		{Type: raftpb.MsgSnap, From: 1, To: 2, Term: 1, Snapshot: raftpb.Snapshot{Metadata: raftpb.SnapshotMetadata{Index: 1000, Term: 1}, Data: data}},
+		{Type: raftpb.MsgSnap, From: 1, To: 2, Term: 1, Snapshot: &raftpb.Snapshot{Metadata: raftpb.SnapshotMetadata{Index: 1000, Term: 1}, Data: data}},
 		{Type: raftpb.MsgHeartbeat, From: 1, To: 2, Term: 1, Commit: 3},
 		{Type: raftpb.MsgHeartbeatResp, From: 1, To: 2, Term: 1},
 	}