raft: make Message.Snapshot nullable, halve struct size

This commit makes the rarely used `raftpb.Message.Snapshot` field nullable.
In doing so, it reduces the memory size of a `raftpb.Message` message from
264 bytes to 128 bytes — a 52% reduction in size.

While this commit does not change the protobuf encoding, it does change
how that encoding is used. `(gogoproto.nullable) = false` instruct the
generated proto marshaling logic to always encode a value for the field,
even if that value is empty. `(gogoproto.nullable) = true` instructs the
generated proto marshaling logic to omit an encoded value for the field
if the field is nil.

This raises compatibility concerns in both directions. Messages encoded
by new binary versions without a `Snapshot` field will be decoded as an
empty field by old binary versions. In other words, old binary versions
can't tell the difference. However, messages encoded by old binary versions
with an empty Snapshot field will be decoded as a non-nil, empty field by
new binary versions. As a result, new binary versions need to be prepared
to handle such messages.

While Message.Snapshot is not intentionally part of the external interface
of this library, it was possible for users of the library to access it and
manipulate it. As such, this change may be considered a breaking change.

Signed-off-by: Nathan VanBenschoten <[email protected]>

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_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

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;

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")

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()
 }