Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explore using vtprotobuf #13107

Closed
lilic opened this issue Jun 15, 2021 · 10 comments
Closed

Explore using vtprotobuf #13107

lilic opened this issue Jun 15, 2021 · 10 comments

Comments

@lilic
Copy link
Contributor

lilic commented Jun 15, 2021

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
Copy link
Contributor

sounds like a good idea

@ptabor
Copy link
Contributor

ptabor commented Jul 13, 2021

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.

karuppiah7890 added a commit to karuppiah7890/issues-info that referenced this issue Jul 13, 2021
@lilic
Copy link
Contributor Author

lilic commented Jul 26, 2021

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.

@lilic lilic self-assigned this Jul 26, 2021
@serathius
Copy link
Member

serathius commented Jul 26, 2021

Can you verify that vtprotobuf supports custom options used in #13216 and needed to implement WAL proto annotation for storage versioning?

@stale
Copy link

stale bot commented Oct 24, 2021

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.

@stale stale bot added the stale label Oct 24, 2021
@lilic lilic removed their assignment Oct 26, 2021
@stale stale bot removed the stale label Oct 26, 2021
@lilic
Copy link
Contributor Author

lilic commented Oct 26, 2021

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

@stale
Copy link

stale bot commented Jan 29, 2022

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.

@stale stale bot added the stale label Jan 29, 2022
@stale stale bot closed this as completed Feb 19, 2022
@serathius serathius reopened this Feb 20, 2022
@stale stale bot removed the stale label Feb 20, 2022
@serathius
Copy link
Member

Created PR that migrates proto generation to vtproto #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
Copy link

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
Copy link

stale bot commented Jul 10, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants