jetcd icon indicating copy to clipboard operation
jetcd copied to clipboard

Provide a solution for Netty version conflicts - grpc-netty requires specific Netty version

Open lhotari opened this issue 1 year ago • 19 comments

Is your feature request related to a problem? Please describe.

grpc-netty is compatible only with specific Netty versions. For example, the officially supported version is 4.1.100.Final at the moment.

grpc provided grpc-netty-shaded package, but that's not compatible with jetcd. There are conflicts with io.grpc.netty.GrpcSslContexts/io.grpc.netty.shaded.io.grpc.netty.GrpcSslContexts and io.netty.handler.ssl.SslContext/io.grpc.netty.shaded.io.netty.handler.ssl.SslContext classes.

Describe the solution you'd like

I'm not sure what solution would be backwards compatible since GrpcSslContext and SslContext classes are exposed in the jetcd client builders.

Describe alternatives you've considered

Additional context

lhotari avatar Jun 12 '24 07:06 lhotari

Those classes have been exposed mainly because of my very limited time, but one of the original design goal of jetcd was to hide internal GRPC/Netty/Guava stuffs so I'd be happy if someone can help designing a replacement for the current ssl setup :)

@lhotari do you by chance have any time ?

lburgazzoli avatar Jun 12 '24 10:06 lburgazzoli

@lhotari do you by chance have any time ?

@lburgazzoli I'd like to help, but unfortunately my time is very limited at the moment. I'm currently working 100%+ on Apache Pulsar and Apache Bookkeeper.

I was able to create a workaround for Apache Pulsar and Apache Bookkeeper for this issue. In Pulsar and Bookkeeper we use maven and I'm using maven-shade-plugin to transform the jetcd-core jar to use grpc-netty-shaded packages instead of grpc-netty. It also requires relocating vertx-grpc to use grpc-netty-shaded packages. This seems to be a sufficient workaround for Pulsar and Bookkeeper. The PR for Bookkeeper is https://github.com/apache/bookkeeper/pull/4426.

For jetcd-core, it would be necessary to hide internals as you mentioned. After that has been completed, it would be possible to provide a shaded artifact that could either shade grpc completely or relocate vertx-grpc and the internal classes of jetcd-core to use grpc-netty-shaded instead of grpc-netty so that there wouldn't be similar Netty compatibility issues in the future.

lhotari avatar Jun 12 '24 15:06 lhotari

ok, I may have some time next week so let see if I can make something not to horrible :) I'll keep you posted

lburgazzoli avatar Jun 12 '24 15:06 lburgazzoli

I have reported the Netty 4.1.111.Final compatibility issue as grpc/grpc-java#11284 . There's a repro case using jetcd-core/jetcd-test 0.8.2, vert-grpc 4.5.8, grpc-java 1.64.0. It is described in the issue comment https://github.com/grpc/grpc-java/issues/11284#issuecomment-2168113850.

lhotari avatar Jun 14 '24 14:06 lhotari

@lhotari do you have an example about how you configure ssl ?

lburgazzoli avatar Jun 17 '24 07:06 lburgazzoli

@lhotari do you have an example about how you configure ssl ?

@lburgazzoli This is the example from Apache Pulsar: https://github.com/apache/pulsar/blob/f3d4d5ac0442eed2b538b8587186cdc0b8df9987/pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/EtcdMetadataStore.java#L138-L148 .

(The Pulsar code is now using the custom shading solution that transforms jetcd-core to use gprc-netty-shaded.)

lhotari avatar Jun 17 '24 08:06 lhotari

So I'm thinking to add a new builder, similar to

https://github.com/etcd-io/jetcd/blob/4d93e3b8fa8244cf17acff2b90c2a4f3b32ed3d2/jetcd-core/src/main/java/io/etcd/jetcd/ClientBuilder.java#L290-L302

but provided as an interface with an internal implementation to hide netty(grpc classes and to make the current setup deprecated for 1 release,

@lhotari would that work for you ?

lburgazzoli avatar Jun 17 '24 09:06 lburgazzoli

but provided as an interface with an internal implementation to hide netty(grpc classes and to make the current setup deprecated for 1 release,

@lhotari would that work for you ?

I think so. I guess one of the implementation problems would be to support gprc-netty and grpc-netty-shaded in the same code base. Have you considered to removing the dependency to grpc-java completely? I learned from @vietj that vertx-grpc provides a new grpc-client that doesn't use grpc-java at all. Docs at https://vertx.io/docs/vertx-grpc/java/#_vert_x_grpc_client

lhotari avatar Jun 17 '24 10:06 lhotari

Have you considered to removing the dependency to grpc-java completely? I learned from @vietj that vertx-grpc provides a new grpc-client that doesn't use grpc-java at all. Docs at https://vertx.io/docs/vertx-grpc/java/#_vert_x_grpc_client

oh, that's very interesting. I was not aware of that so I'll have a look because well, grpc-java is an immense source of troubles.

lburgazzoli avatar Jun 17 '24 10:06 lburgazzoli

mh, the main issue with the vert.x client is that it seems it does not support any load balancing option and endpoint discovery, at least I've not been able to find it.

@lhotari @vietj is there any way to achieve it ?

lburgazzoli avatar Jun 17 '24 12:06 lburgazzoli

@lburgazzoli we have this coming in vertx 5 based on our own implementation actually (https://github.com/eclipse-vertx/vertx-service-resolver)

vietj avatar Jun 17 '24 14:06 vietj

@lburgazzoli we have this coming in vertx 5 based on our own implementation actually (https://github.com/eclipse-vertx/vertx-service-resolver)

oh, that's great! do you by chance have an example of the integration with the vertx-grpc-client ?

lburgazzoli avatar Jun 17 '24 14:06 lburgazzoli

yes there is an integration since this is integrated with the vertx http client on which the grpc client is built.

here is how the HttpClient is created with a load balancer : https://github.com/eclipse-vertx/vert.x/blob/master/src/main/java/examples/HTTPExamples.java#L1405

the gRPC client wraps actually this client so we need in the gRPC client API to provide a similar buildr API

vietj avatar Jun 17 '24 19:06 vietj

which is this issue : https://github.com/eclipse-vertx/vertx-grpc/issues/100

vietj avatar Jun 17 '24 19:06 vietj

Currently it can be achieved with https://github.com/eclipse-vertx/vertx-grpc/blob/main/vertx-grpc-client/src/main/java/io/vertx/grpc/client/GrpcClient.java#L95 which allows to wrap a client, we need to provide an integrated builder API instead

vietj avatar Jun 17 '24 19:06 vietj

It seems switching to vert.x grpc client requires some more work than adding a builder, so I've opened a new issue https://github.com/etcd-io/jetcd/issues/1372

lburgazzoli avatar Jun 18 '24 07:06 lburgazzoli

  • Just to give my opinion, if io.grpc:grpc-netty-shaded is used, it will cause troubles for users using jetcd under GraalVM Native Image, which is reported as https://github.com/oracle/graalvm-reachability-metadata/issues/377 . The source of this matter is that Netty 4 is unwilling to drop support for GraalVM Native Image on CentOS 6.
  • As known background, apache/shardingsphere writes unit tests for io.etcd:jetcd-core:0.7.7 under GraalVM Native Image compiled with GraalVM CE For JDK 22.0.2.

linghengqian avatar Aug 08 '24 04:08 linghengqian

At this stage, the plan is to switch to vertx-grpc-client

lburgazzoli avatar Aug 08 '24 06:08 lburgazzoli

@lhotari I've released version 0.8.3 with updated netty, the switch to vertx client would require some time since I'm running very low of spare time

lburgazzoli avatar Aug 16 '24 13:08 lburgazzoli