Skip to content

Commit

Permalink
raft: disable XXX_NoUnkeyedLiteral, XXX_unrecognized, and XXX_sizecac…
Browse files Browse the repository at this point in the history
…he fields in protos

This commit removes the `XXX_NoUnkeyedLiteral`, `XXX_unrecognized`, and
`XXX_sizecache` auto-generated fields from generated protobuf structs in
the raft package. This was done for all of the same reasons CockroachDB
removed the generation of these fields in cockroachdb/cockroach#38404.
They come with very limited advantages but moderate disadvantages.

`XXX_NoUnkeyedLiteral` and `XXX_sizecache` were only enabled recently in
cc7b4fa, and this appears to have been unintentional. Meanwhile,
`XXX_unrecognized` has been around for longer and has arguably more
reason to stay because it can assist with forwards compatibility.
However, any real mixed-version upgrade story for this package is mostly
untold at this point, and keeping this field seems just as likely to
cause unexpected bugs (e.g. a field was propagated but not updated
correctly when passed through an old version) as it seems to fix real
issues, so it also doesn't warrant its cost.

This reduces the in-memory representation size of all Raft protos.
Notably, it reduces the memory size of an `Entry` proto from *80 bytes*
to *48 bytes* and the memory size of a `Message` proto from *392 bytes*
to *264 bytes*. Both of these structs are used frequently, and often in
slices, where this wasted space really starts to add up.

This was motivated by a regression in microbenchmarks in CockroachDB due
to cc7b4fa, which was caught in cockroachdb/cockroach#62212.
  • Loading branch information
nvanbenschoten committed Mar 20, 2021
1 parent ebc0174 commit e51c697
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 203 deletions.
1 change: 0 additions & 1 deletion raft/raftpb/confstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ func (cs ConfState) Equivalent(cs2 ConfState) error {
s(&cs.Learners)
s(&cs.VotersOutgoing)
s(&cs.LearnersNext)
cs.XXX_unrecognized = nil
}

if !reflect.DeepEqual(cs1, cs2) {
Expand Down
Loading

0 comments on commit e51c697

Please sign in to comment.