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

Update to Protobuf 4.x #11015

Open
neeme-praks-sympower opened this issue Mar 14, 2024 · 13 comments
Open

Update to Protobuf 4.x #11015

neeme-praks-sympower opened this issue Mar 14, 2024 · 13 comments
Assignees
Labels
enhancement gRPC 2.0 Issue requires breaking stable APIs
Milestone

Comments

@neeme-praks-sympower
Copy link

Is your feature request related to a problem?

I would like to use the latest released version of Protobuf (4.26.0) but I cannot as gRPC is still using the old version.

Describe the solution you'd like

Update Protobuf dependency and make it work with the new version.

Describe alternatives you've considered

I hoped that the update would be easy (just a version bump) so I tried doing it myself but it resulted in a compilation failure so I'll leave it to you to figure it out.

The failure:

> Task :grpc-protobuf:compileJava FAILED
grpc-java/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java:38: error: cannot access GeneratedMessageV3
          ProtoLiteUtils.metadataMarshaller(com.google.rpc.Status.getDefaultInstance()));
                                                                 ^
  class file for com.google.protobuf.GeneratedMessageV3 not found
grpc-java/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java:171: error: cannot access Builder
        .setCode(status.getCode().value());
        ^
  class file for com.google.protobuf.GeneratedMessageV3$Builder not found
2 errors
@ganadist
Copy link

ganadist commented Mar 15, 2024

https://protobuf.dev/news/2023-12-05

Also, I'm wondering how this protobuf changes will affect

  • gRPC public API changes
  • Compatibility with artifacts which were generated by old protobuf/gRPC compilers.

@ejona86 ejona86 added the gRPC 2.0 Issue requires breaking stable APIs label Mar 15, 2024
@ejona86
Copy link
Member

ejona86 commented Mar 15, 2024

The protobuf breakage is pretty serious and goes against wisdom like https://jlbp.dev/JLBP-7 . I don't see how the community can absorb it. googleapis libraries are hit even harder than we are, so I think we'd wait until they have an idea how to deal with this (otherwise they couldn't upgrade to newer grpc versions). I don't expect that to be too big of a problem for the community, as I think most protobuf-using projects will be in shock.

I'm in discussions with others, but I'd expect the process to figure this out to take months.

@augi
Copy link

augi commented Mar 17, 2024

After updating to Protobuf 4.26.0, we observe this issue:

java.lang.NoClassDefFoundError: com/google/protobuf/GeneratedMessageV3
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1017)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862)
	at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525)
	at io.grpc.health.v1.HealthGrpc.getCheckMethod(HealthGrpc.java:38)
	at io.grpc.health.v1.HealthGrpc.getServiceDescriptor(HealthGrpc.java:413)
	at io.grpc.health.v1.HealthGrpc.bindService(HealthGrpc.java:350)
	at io.grpc.health.v1.HealthGrpc$HealthImplBase.bindService(HealthGrpc.java:169)

The announcement of the Protobuf release contains this: [Java] The base class for generated messages will be GeneratedMessage, not GeneratedMessageV3.

So I guess this has to be adjusted.

@suztomo
Copy link
Contributor

suztomo commented Mar 22, 2024

@neeme-praks-sympower Does any library or dependency require you to upgrade Protobuf runtime version to 4.x?

@be-hase
Copy link

be-hase commented Mar 22, 2024

It seems that proto-google-common-protos, which grpc-protobuf depends on, is not yet ver4 compliant.
https://github.com/googleapis/sdk-platform-java/tree/main/java-common-protos

guilgaly added a commit to gatling/gatling-grpc-demo that referenced this issue Mar 26, 2024
Motivation:

- grpc-java is not fully compatible with Protobuf 4; see: grpc/grpc-java#11015
- Protobuf 4 breaks our demo server due to an incompatibility between the generated code and what the grpc-java library expects.

Modifications:

- Downgrade to Protobuf 3 on all projects.
- Prevent upgrade to Protobuf 4 in dependabot.yml (note: scala-steward doesn't seem to pick up Protobuf updates anyway, probably because it's not configured as a library dependency per se).

Result:

Stick to Protobuf 3 for now.
guilgaly added a commit to gatling/gatling-grpc-demo that referenced this issue Mar 26, 2024
Motivation:

- grpc-java is not fully compatible with Protobuf 4; see: grpc/grpc-java#11015
- Protobuf 4 breaks our demo server due to an incompatibility between the generated code and what the grpc-java library expects.

Modifications:

- Downgrade to Protobuf 3 on all projects.
- Prevent upgrade to Protobuf 4 in dependabot.yml (note: scala-steward doesn't seem to pick up Protobuf updates anyway, probably because it's not configured as a library dependency per se).

Result:

Stick to Protobuf 3 for now.
guilgaly added a commit to gatling/gatling-grpc-demo that referenced this issue Mar 26, 2024
Motivation:

- grpc-java is not fully compatible with Protobuf 4; see: grpc/grpc-java#11015
- Protobuf 4 breaks our demo server due to an incompatibility between the generated code and what the grpc-java library expects.

Modifications:

- Downgrade to Protobuf 3 on all projects.
- Prevent upgrade to Protobuf 4 in dependabot.yml (note: scala-steward doesn't seem to pick up Protobuf updates anyway, probably because it's not configured as a library dependency per se).

Result:

Stick to Protobuf 3 for now.
@krickert
Copy link

Why couldn't grpc just bring back generated message v3? The classes appear almost identical, I suppose it can just extend it?

I tried that myself - it somewhat worked. I got past the generated message v3 error, but I was getting serializaiton errors everywhere. I think they may have changed the serializaiton spec too?

Anyway, I'm looking forward to this fix. I might try this again today.

@neeme-praks-sympower
Copy link
Author

@neeme-praks-sympower Does any library or dependency require you to upgrade Protobuf runtime version to 4.x?

No. We just try to keep our dependencies up-to-date.

@suztomo
Copy link
Contributor

suztomo commented Apr 1, 2024

@neeme-praks-sympower Thank you.

@ummels
Copy link

ummels commented Apr 9, 2024

I've opened an issue at Protobuf for restoring GeneratedMessageV3 as an alias for GeneatedMessage.

@krickert
Copy link

krickert commented Apr 9, 2024

I've opened an issue at Protobuf for restoring GeneratedMessageV3 as an alias for GeneatedMessage.

I didn't think you're going to win that though

It was a major upgrade with breaking changes. They deprecated V3 awhile ago, better to fix instead of bringing it back. It'll take some time, but GRPC isn't always caught up to the latest protocol buffers.

@snazy
Copy link

snazy commented Apr 9, 2024

The protobuf v4 release effectively breaks a ton of projects. Sure, it's a breaking change - but it is a breaking change without a migration path. The protobuf code generators generated GeneratedMessageV3 and that is now gone - so the tons of existing and working projects that use the protobuf code generators will stop working with protobuf v4.

IMHO, a breaking change must first provide a properly communicated migration path and then give all users and projects enough time to adopt.

Bringing back GeneratedMessageV3 as a migration path SGTM - just not sure how good that'll work.

@ummels
Copy link

ummels commented Apr 9, 2024

I'm in for a fight. 😂

Anyway, if they deprecated V3 a while ago, how come code generated with protoc 3.25.3 still relies on GeneratedMessageV3? Doesn't seem like a smooth transition to me. 🤔

@ejona86
Copy link
Member

ejona86 commented Apr 11, 2024

Quick update: protobuf understands the problem and are considering solutions, and they've said as much publicly. I expect a few more weeks to settle on the approach and then some time to make sure the details are right.

For those watching at home and want to be prepared, use protobuf-java 3.25 and protoc 25. Also, if you have any non-generated code using GeneratedMessageV3, replace those usages with Message. Even if GeneratedMessageV3 is added back to protobuf 4.x the newer generated code won't implement it. It was never intended to be referenced outside of generated code. I'm not aware of any reason to reference GeneratedMessageV3 in hand-written code, so it is hopefully easy to avoid.

(I'm assuming there's no references to GeneratedMessage and protoc 2 since 3.0 came out in 2016. Any expected solution is expected to drop support for the 2.x generated code.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement gRPC 2.0 Issue requires breaking stable APIs
Projects
None yet
Development

No branches or pull requests

10 participants