Change Gossip to send only protobufs

[petermattis]

[tamird]

Brought here by #1633. What's the rationale here?

[bdarnell]

All the usual reasons for choosing protobufs over gob: it's better to be explicit about what's going over the wire and have clear forwards- and backwards-compatibility semantics, protobufs are faster, and it's consistent with everything else we do.

[tamird]

Are you sure protobufs are faster? My last benchmark suggested gob was
faster.
On Jul 4, 2015 10:35 AM, "Ben Darnell" notifications@github.com wrote:

All the usual reasons for choosing protobufs over gob: it's better to be
explicit about what's going over the wire and have clear forwards- and
backwards-compatibility semantics, protobufs are faster, and it's
consistent with everything else we do.

—
Reply to this email directly or view it on GitHub
#629 (comment)
.

[bdarnell]

Your email from June 2 indicated that you were testing golang/protobuf, not gogo/protobuf. Gogo (with generated marshal/unmarshal functions) should be faster.

[petermattis]

In addition to Ben's other points, Gob is being used inappropriately here. Gob encoding/decoding is intended to be connection oriented. That is, you create a gob encode/decoder when the connection is opened and the encoder/decoder cache type info from the other side. They way gossip is using gob it is creating a new encoder/decoder each data needs to be sent/received. This usage pattern likely substantially reduces gob performance.

[thschroeter]

Happy to tackle this.

[tamird]

@thschroeter please do. Let me know if you have any questions.

[thschroeter]

@tamird, do you have a hint regarding encoding the polymorphic payload Val of gossip.info?

Will try to use the oneof feature of protobuf and enumerate all concrete use cases. Looking at the call sites of AddInfo, Val is currently used to propagate int64, string, and proto.StoreDescriptor. I might be missing use cases, though.

I've defined a new proto.GossipInfoStore, to be able to send the delta in GossipRequest as protobuf. Once I know how to encode Val of gossip.info I will open a PR for review.

[tamird]

proto.GossipInfoStore is one option, with oneof as you suggest.

Another option (since the types of each key must be statically known anyway) is to use known encodings for each of the types and store as []byte:

• int64: http://godoc.org/encoding/binary#PutVarint
• string: just convert to a byte array
• proto.StoreDescriptor: Marshal

This may be slightly more ergonomic, but you'll have to try and see.