Pass Message as pointer

[ahrtr]

part of etcd-io/etcd#14533

[Copilot]

Pull request overview

This PR is part of etcd-io/etcd#14533 and migrates raft’s message handling from pb.Message values to *pb.Message pointers across core APIs, internal queues, and test utilities, reducing copying and aligning with protobuf pointer-heavy structures.

Changes:

• Updated public and internal APIs to accept/return *pb.Message (e.g., Node.Step, RawNode.Step, raft.Step, Ready.Messages).
• Converted internal message queues and storage-thread “work” message plumbing to pointer slices ([]*pb.Message).
• Removed raftpb.MessageSliceToPointers/FromPointers helpers after switching callers to pointers.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
util.go	DescribeMessage now takes *pb.Message; recursive formatting updated for pointer responses.
read_only.go	Read-index tracking stores *pb.Message requests.
rawnode.go	RawNode.Step now takes *pb.Message; async storage messages now built as pointers; stepsOnAdvance stores pointers.
rawnode_test.go	Updated adapter and tests/bench to use *pb.Message.
rafttest/node.go	Test node network buffering updated to []*raftpb.Message.
rafttest/network.go	Network interface and queues switched to pointer messages; cloning uses marshal/unmarshal into *Message.
rafttest/network_test.go	Updated network tests to send *raftpb.Message.
rafttest/interaction_env.go	Interaction env in-flight and work message slices switched to []*pb.Message.
rafttest/interaction_env_handler_stabilize.go	Message splitting/local-message checks updated for pointer messages.
rafttest/interaction_env_handler_send_snapshot.go	Snapshot message constructed as *raftpb.Message.
rafttest/interaction_env_handler_process_apply_thread.go	Debug output and message enqueueing updated for pointer messages.
rafttest/interaction_env_handler_process_append_thread.go	Debug output and message enqueueing updated for pointer messages.
rafttest/interaction_env_handler_deliver_msgs.go	Delivery path updated to use []*raftpb.Message.
raftpb/util.go	Removed Message slice pointer/value conversion helpers.
raft.go	Core raft message queues and send/step paths migrated to *pb.Message.
raft_test.go	Test harness/stateMachine interface and network hooks migrated to *pb.Message.
raft_snap_test.go	Snapshot-related tests updated to step *pb.Message.
raft_paper_test.go	Paper tests migrated to *pb.Message and pointer message slices.
raft_flow_control_test.go	Flow control tests updated to step *pb.Message.
node.go	Public Node.Step and Ready.Messages migrated to pointers; internal channels carry *pb.Message.
node_test.go	Node tests migrated to pointer messages and pointer-based msg hooks.
example_test.go	Example helper sendMessages updated to accept []*pb.Message.
doc.go	Documentation example updated to pass *raftpb.Message to Node.Step.

Comments suppressed due to low confidence (2)

rawnode.go:125

• RawNode.Step now accepts *pb.Message but dereferences m without checking for nil. A nil message will cause a panic. Add an explicit nil guard at the top of this method (returning a dedicated error) and consider documenting the non-nil/immutability requirement in the Step comment.

// Step advances the state machine using the given message.
func (rn *RawNode) Step(m *pb.Message) error {
	// Ignore unexpected local messages receiving over network.
	if IsLocalMsg(m.GetType()) && !IsLocalMsgTarget(m.GetFrom()) {
		return ErrStepLocalMsg
	}
	if IsResponseMsg(m.GetType()) && !IsLocalMsgTarget(m.GetFrom()) && rn.raft.trk.Progress[m.GetFrom()] == nil {
		return ErrStepPeerNotFound
	}
	return rn.raft.Step(m)

raft_test.go:583

• tt.msgs is typed as []*pb.Message, but this slice literal contains pb.Message values (missing '&'), which will not compile. Change the elements to pointers (e.g., &pb.Message{...}).

			newNetwork(nil, nil, nil),
			[]*pb.Message{
				{From: new(uint64(1)), To: new(uint64(1)), Type: pb.MsgProp.Enum(), Entries: []*pb.Entry{{Data: []byte("somedata")}}},
				{From: new(uint64(1)), To: new(uint64(2)), Type: pb.MsgHup.Enum()},
				{From: new(uint64(1)), To: new(uint64(2)), Type: pb.MsgProp.Enum(), Entries: []*pb.Entry{{Data: []byte("somedata")}}},
			},

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ahrtr]

cc @fuweid @serathius

[fuweid]

LGTM

After this change, raft messages are carried as pointers. I checked etcd's Ready handling path: etcd does mutate Ready.Messages in processMessages, for example by setting To = 0 to drop messages. On the raft side, Ready/acceptReady clears r.msgs and r.msgsAfterAppend after the Ready is accepted, so I don't see a current raft-side retained-pointer race there.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, fuweid, serathius

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [ahrtr,serathius]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ahrtr]

On the raft side, Ready/acceptReady clears r.msgs and r.msgsAfterAppend after the Ready is accepted, so I don't see a current raft-side retained-pointer race there.

Also raft doesn't reuse any message.