Using Pub/Sub JAVA client in play framework 2.5 - netty conflicts

[thefunkjunky]

Issue summary: Client's services are built on top of the play framework 2.5. Some of their services need to publish messages to pub/sub, but the Google Pub/Sub java api depends on netty 4.1.x, which conflicts with netty 4.0.x used by play framework 2.5.
Is there a solution to get around this?

[mziccard]

Argh @thefunkjunky I need to be honest with you, I was expecting this issue...

This is a problem with gRPC (a dependency of ours) and Play 2.5. gRPC depends on netty 4.1, while Play 2.5 depends on netty 4.0. Unfortunately, these 2 netty versions are not binary compatible.

A possible solution (see stackoverflow) could be downgrading to Play 2.4.6 which uses netty 3.8.0 that had a completely different project structure and package names (hence no name clashes).

Play 2.6 should ship with netty 4.1 and possibly solve this issue.

@garrettjonesgoogle do you know whether gRPC people considered shading some of their dependencies? There seem to be a number of people encountering this compatibility issues.

[garrettjonesgoogle]

I don't know - @ejona86 , can you comment?

[ejona86]

So shading is a fine strategy, but since Netty is exposed through our API (e.g., you can provide your own EventLoopGroup to NettyChannelBuilder), gRPC project pre-shading can cause problems.

[garrettjonesgoogle]

We may have to shade all of gRPC in google-cloud-java. There are two problematic dependencies:

1. Guava
2. Netty (also see: Netty incompatibility when using Pub/Sub with Dataproc #1522 - Netty conflict with Spark Google Dataproc)

[ejona86]

I have since made grpc/grpc-java#2485. It's not submitted yet, and if you use NettyChannelBuilder directly it doesn't work well, but we can maybe push harder on that.

Guava is a different story. Would I be right to assume issues are caused by the ancient guava-jdk5 used by the v1 client libraries?

[garrettjonesgoogle]

The issue with Guava isn't the guava-jdk5 dependency, it is a larger issue with the Guava deprecation policy. Basically as soon as we pick a particular version of Guava and expose types from it, that means no one can mix our library with a version of Guava that is 2 years older or 2 years newer than the version we picked. There are a fair number of consumers that this doesn't work for, e.g. Hadoop which is stuck on Guava 11.

[ejona86]

and expose types from it

You can't do that at all, if you are going to shade. So for example, it is hard to shade Guava for gRPC when the Future stub is in play, because it uses ListenableFuture but if we shade ListenableFuture then none of the normal utilities will work and users have to use our shaded version.

[garrettjonesgoogle]

Our plan was to create an interface analogous to ListenableFuture called https://github.com/googleapis/gax-java/blob/master/src/main/java/com/google/api/gax/grpc/RpcFuture.java , which allows users to add callbacks, and make a bridge from ListenableFuture to RpcFuture called https://github.com/googleapis/gax-java/blob/master/src/main/java/com/google/api/gax/grpc/ListenableFutureDelegate.java (which is package-private). What do you mean by "the normal utilities?"

[ejona86]

Hadoop which is stuck on Guava 11.

I think the argument there is that Hadoop should be shading/using class loading magic (a la containers). Because without doing so nobody can mix Hadoop and Guava. But I do understand the general "exposing a dependency on Guava can cause problems" argument.

Our plan was to create an interface analogous to ListenableFuture called RpcFuture

Okay. And then your users can create their own utility to convert that to a ListenableFuture.

What do you mean by "the normal utilities?"

I mean things like cgc.util.concurrent.Futures. There are a lot of generic utility methods that do things with ListenableFutures.

make a bridge from ListenableFuture to RpcFuture called ListenableFutureDelegate

That's an implementation detail, right? The user of your library doesn't care about that at all, right?

Note that any reason you are using ListenableFuture in your code could be an example of why your users would want ListenableFuture.

[garrettjonesgoogle]

Hadoop which is stuck on Guava 11.

I think the argument there is that Hadoop should be shading/using class loading magic (a la containers). Because without doing so nobody can mix Hadoop and Guava. But I do understand the general "exposing a dependency on Guava can cause problems" argument.

Hadoop's problem isn't just that it depends on Guava, it's that it made Guava part of its surface, so if it upgrades to a newer version of Guava, it's a breaking change for its users. If it was only an implementation detail, they could have done something like shading Guava a long time ago.

Our plan was to create an interface analogous to ListenableFuture called RpcFuture

Okay. And then your users can create their own utility to convert that to a ListenableFuture.

I was considering creating a "bridge" project that depends on Guava and connects GAX features to Guava, so that each user doesn't have to do it themselves.

What do you mean by "the normal utilities?"

I mean things like cgc.util.concurrent.Futures. There are a lot of generic utility methods that do things with ListenableFutures.

Right, but people don't have to use those utilities. This could be addressed with the "bridge" project I mentioned above.

make a bridge from ListenableFuture to RpcFuture called ListenableFutureDelegate

That's an implementation detail, right? The user of your library doesn't care about that at all, right?

That is correct.

Note that any reason you are using ListenableFuture in your code could be an example of why your users would want ListenableFuture.

The key thing that comes out of ListenableFuture is the capability of adding a callback so that a thread isn't consumed. I don't think the key is so much that exact interface as that we provide the callback capability.

[kuroneko25]

I ran into a similar issue where I use grpc-all 1.1.2 and google-cloud-pubsub in the same project. Is this going to be addressed by grpc/grpc-java#2485?

Specifically this exception

Exception in thread "main" java.lang.NoSuchMethodError: io.grpc.netty.NettyChannelBuilder.build()Lio/grpc/internal/ManagedChannelImpl;
    at com.google.api.gax.grpc.InstantiatingChannelProvider.createChannel(InstantiatingChannelProvider.java:119)
    at com.google.api.gax.grpc.InstantiatingChannelProvider.getChannel(InstantiatingChannelProvider.java:106)
    at com.google.api.gax.grpc.ProviderManager.getChannel(ProviderManager.java:106)
    at com.google.api.gax.grpc.ChannelAndExecutor.create(ChannelAndExecutor.java:67)
    at com.google.api.gax.grpc.ClientSettings.getChannelAndExecutor(ClientSettings.java:78)
    at com.google.cloud.pubsub.spi.v1.PublisherClient.<init>(PublisherClient.java:182)
    at com.google.cloud.pubsub.spi.v1.PublisherClient.create(PublisherClient.java:173)

[pongad]

If I understand this correctly, the answer is yes.

The exception is NoSuchMethod not NoClassDef. So I'm guessing

1. grpc and gax depends on different versions
2. grpc's version takes precedence
3. grpc's version doesn't have the build() method we need

Once that issue is addressed, things will work together again.

ccing @garrettjonesgoogle to make sure I'm diagnosing this correctly.

[garrettjonesgoogle]

@pongad , that analysis looks correct to me.

[ejona86]

No, grpc/grpc-java#2485 would not fix your issue. That issue is not related to netty itself but the netty transport in gRPC. It looks like gRPC broke an ABI (it is still source compatible) by moving the location of build(). However, NettyChannelBuilder is explicitly stated to be unstable API, so GAX shouldn't be using it or would need to shade all of grpc.

[garrettjonesgoogle]

@ejona86 So does this fairly characterize the situation?:

gRPC is GA, but the only practical general-purpose transport is Netty, and the only way to use that transport is to use an @Experimental class, so for all practical purposes if you use gRPC, you will be broken between minor versions despite your best intentions.

[ejona86]

the only way to use that transport is to use an @experimental class,

I don't agree with that. If you're talking about NettyChannelBuilder, then no, you shouldn't use it in a library. Instead, you should use ManagedChannelBuilder's forAddress(String, int) or forTarget(String).

[garrettjonesgoogle]

Oh! I don't know how we ended up using NettyChannelBuilder instead of ManagedChannelBuilder. We will switch it.

[garrettjonesgoogle]

googleapis/gax-java#208

[garrettjonesgoogle]

I wanted to figure out where we're at for the Netty conflicts. (Guava doesn't actually seem to be relevant to the original requester, so I won't discuss that any more here.)

Gax:

In googleapis/gax-java#215, we have switched from NettyChannelBuilder to ManagedChannelBuilder (we just haven't created an artifact with it yet).

I did a grep for netty in gax-java, and it looks like there are more netty class references there that need to be changed. I have filed issue googleapis/gax-java#234 to address this.

gRPC:

@ejona86 , it looks like https://github.com/grpc/grpc-java/pull/2485/files isn't submitted yet. Is there a timeline for it?

[ejona86]

No, there's not a timeline; I've started an email chain to discuss prioritization. Today the solution would be for users to shade it themselves. Based on other discussions, it also sounds like the PR will be incomplete because it fails to address tcnative loading issues. But I guess that will be solved separately by netty/netty#6271 and may not impact as many users.

[kuroneko25]

So if we want to shade ourselves what is the best example that we can look at? What are the artifacts/classes that need to be shaded? (we use maven)

[pongad]

According to @garrettjonesgoogle , the latest release should shade netty and this should no longer be a problem.

Due to age of issue, I'll close this, but please either reopen or open a new issue if you continue to experience problems.