Explore using vtprotobuf

etcd-io / etcd

Distributed reliable key-value store for the most critical data of a distributed system

https://etcd.io

Apache License 2.0

47.62k stars 9.75k forks source link

Explore using vtprotobuf #13107

Closed lilic closed 1 year ago

lilic commented 3 years ago

Recently the Vitess folks have posted a blog on their optimizations on protobuf generator https://vitess.io/blog/2021-06-03-a-new-protobuf-generator-for-go/. The project source is located at https://github.com/planetscale/vtprotobuf.

The findings and benchmarks were very interesting and impressive, and it would be interesting to try it out and benchmark and compare the differences. Any thoughts against this?

Opening this issue mainly to get the conversation started as I believe it would be good to find alternatives as the gogo/protobuf project is looking for new maintainers, so we either need to take that on, or consider alternatives.

wilsonwang371 commented 3 years ago

sounds like a good idea

ptabor commented 3 years ago

Makes sense. It seems that etcd/client/v3 (especially op.go) pretty well abstracts etcd client surface (v3) from actual proto-generated API, so it should be possible to upgrade to proto-v2 compatible proto surface without breaking end-users.

lilic commented 3 years ago

so it should be possible to upgrade to proto-v2 compatible proto surface without breaking end-users.

Thanks! I should be able to get to this for this release.

serathius commented 3 years ago

Can you verify that vtprotobuf supports custom options used in https://github.com/etcd-io/etcd/pull/13216 and needed to implement WAL proto annotation for storage versioning?

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

lilic commented 2 years ago

I didn't have time to look into this yet, so leaving up to anyone else who is interested.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

serathius commented 2 years ago

Created PR that migrates proto generation to vtproto https://github.com/etcd-io/etcd/pull/13719 There is still work related to:

Update code to match generated go files (name changes, pointer changes etc)
Verify if there are any breaking changes due to dropped gogo annotations

johanbrandhorst commented 2 years ago

For yet another alternative to GoGo protobuf, see https://github.com/CrowdStrike/csproto. This was developed by Crowdstrike to fill the performance gap left between GoGo and the official Go protobuf implementation. It can also be used to migrate from APIv1 to APIv2 over time:

The runtime-agnostic APIs do the work of calling the correct runtime based on which generator was used to emit the Go struct so you don't run into problem when, for example, you pass a struct generated by protoc-gen-gogo to proto.Unmarshal() from google.golang.org/protobuf/proto. csproto.Unmarshal() inspects the underlying type of the msg parameter and will call Gogo's proto.Unmarshal() instead.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.