storage: avoid map lookup in raftRcvdMessages, replace with array

[nvanbenschoten]

Drive by cleanup with small perf impact.

Release note: None

[cockroach-teamcity]

This change is 

[petermattis]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @nvanbenschoten)

---

pkg/storage/raft.go, line 28 at r1 (raw file):

func init() {
	if numRaftMsgTypes != len(raftpb.MessageType_name) {

I wonder if this should instead look for the max-raft-message-type in raftpb.MessageType_name. The current code will fail if a new message type is added and an old one is removed.

[nvanbenschoten]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @petermattis)

---

pkg/storage/raft.go, line 28 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

I wonder if this should instead look for the max-raft-message-type in raftpb.MessageType_name. The current code will fail if a new message type is added and an old one is removed.

Is that really a concern? The order of the message types in the array doesn't need to be stable across builds, as long as all of the references to the message types remain. And if not, we'll already hit a compilation error elsewhere.

[petermattis]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @nvanbenschoten)

---

pkg/storage/raft.go, line 28 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Is that really a concern? The order of the message types in the array doesn't need to be stable across builds, as long as all of the references to the message types remain. And if not, we'll already hit a compilation error elsewhere.

The MessageType enum values are encoded in the proto. My point was what happens if someone adds MsgBuyMeABeer = 19 and removes MsgCheckQuorum. The number of elements in MessageType_name remains at 19, but raftRcvdMessages[MsgBuyMeABeer] is an out-of-bounds slice access.

[nvanbenschoten]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner and @petermattis)

---

pkg/storage/raft.go, line 28 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

The MessageType enum values are encoded in the proto. My point was what happens if someone adds MsgBuyMeABeer = 19 and removes MsgCheckQuorum. The number of elements in MessageType_name remains at 19, but raftRcvdMessages[MsgBuyMeABeer] is an out-of-bounds slice access.

Ok, I see what you mean. How's that?

[petermattis]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner and @nvanbenschoten)

---

pkg/storage/raft.go, line 28 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Ok, I see what you mean. How's that?

Looks good to me. Thanks.

[nvanbenschoten]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)