search

apache / pulsar

Apache Pulsar - distributed pub-sub messaging system
https://pulsar.apache.org/
Apache License 2.0
14.1k stars 3.56k forks source link

[Bug] Version conflicts with the protobuf inside the pulsar client #22263

Open pqab opened 5 months ago

pqab commented 5 months ago

Search before asking

Version

Pulsar client & client admin 3.0.2

Minimal reproduce step

We have grpc application which is using protobuf 3.25.x, however when we build the proto files, the protobuf packed inside the pulsar client which is using an old version override our dependencies, causing issues in the grpc runtime envrionment, even if we tried to exclude from the gradle, it doesn't works, because it built inside the client jar directly

What did you expect to see?

The protobuf library inside the pulsar client shouldn't override our dependencies

What did you see instead?

It took priority to load the library from the pulsar client

Anything else?

We have a workaround to use pulsar-client-original & pulsar-client-admin-original client instead

Are you willing to submit a PR?

dao-jun commented 5 months ago

Try to use pulsar-client-all

dao-jun commented 5 months ago 
<!-- https://mvnrepository.com/artifact/org.apache.pulsar/pulsar-client-all -->
<dependency>
    <groupId>org.apache.pulsar</groupId>
    <artifactId>pulsar-client-all</artifactId>
    <version>3.0.3</version>
</dependency>

instead of 

<dependency>
    <groupId>org.apache.pulsar</groupId>
    <artifactId>pulsar-client</artifactId>
    <version>3.0.3</version>
</dependency>
<!-- https://mvnrepository.com/artifact/org.apache.pulsar/pulsar-client-admin -->
<dependency>
    <groupId>org.apache.pulsar</groupId>
    <artifactId>pulsar-client-admin</artifactId>
    <version>3.0.3</version>
</dependency>

pulsar-client-all will shade these deps.

pqab commented 5 months ago

image

pulsar-client-all has the same issue, the problem is the protobuf library inside the pulasr client is overriding our dependency, is it possible to decouple the protobuf from the client jar?

lhotari commented 5 months ago

I believe that the unshaded dependency has the artifact Id pulsar-client-original. Use should be able to override the version of protobuf when that is used.

dao-jun commented 5 months ago

pulsar-client-all skipped shade protobuf-java And protobuf-java introduced by pulsar-client-origin. Maybe

    <dependency>
      <groupId>org.apache.pulsar</groupId>
      <artifactId>pulsar-client-original</artifactId>
      <version>3.0.3</version>
      <exclusions>
        <exclusion>
          <groupId>com.google.protobuf</groupId>
          <artifactId>protobuf-java</artifactId>
        </exclusion>
      </exclusions>
    </dependency>
    <dependency>
      <groupId>org.apache.pulsar</groupId>
      <artifactId>pulsar-client-admin-original</artifactId>
      <version>3.0.3</version>
      <exclusions>
        <exclusion>
          <groupId>org.apache.pulsar</groupId>
          <artifactId>pulsar-client-original</artifactId>
        </exclusion>
      </exclusions>
    </dependency>

can works

pqab commented 5 months ago

yes, that's what we are using now we understand you shaded dependencies into the pulsar-client, if you move the shaded dependencies to under a shaded package or changing the namespace, then it won't affects the end user, we can use our own version of the protobuf with the pulsar-client

lhotari commented 5 months ago

yes, that's what we are using now we understand you shaded dependencies into the pulsar-client, if you move the shaded dependencies to under a shaded package, then it won't affects the end user, we can use our own version of the protobuf with the pulsar-client

This issue with protobuf in Pulsar client is similar to the problems with Avro (#9718, #13096, #21230) and Jackson (#6528) where you need to use pulsar-client-original dependency. I agree that this is not optimal.

Just a reminder that "we" and "you" are the same in an open source project. It's "we", the community, that can impact the development and improvements to Apache Pulsar. Contributions are welcome! Subscribing to the dev mailing list and joining the #dev channel in Slack are ways to get connected with the Apache Pulsar community.

lhotari commented 5 months ago

There's a slightly unrelated issue in Pulsar in upgrading the protobuf version that Pulsar uses internally between Pulsar and Bookkeeper. The version has to be the same in Pulsar and Bookkeeper currently. Dev mailing list thread is https://lists.apache.org/thread/s9g6w31vtwzgqf162hhlcr2nx3y68gv5 . It points to the Bookkeeper mailing list thread which contains more details.

lhotari commented 5 months ago

Regarding contributions, I think that a useful first contribution would be to update the documentation for Pulsar Java client to include the information about the workarounds for using a custom version of protobuf.

dao-jun commented 5 months ago

image image

I'm wondering why do we skipped shade protobuf-java in pulsar-client, pulsar-client-admin and pulsar-client-all? Can we just shade it? @lhotari

pqab commented 5 months ago

Regarding contributions, I think that a useful first contribution would be to update the documentation for Pulsar Java client to include the information about the workarounds for using a custom version of protobuf.

we will take some time to check the documentation, and might try to create some PRs later

lhotari commented 5 months ago

I'm wondering why do we skipped shade protobuf-java in pulsar-client, pulsar-client-admin and pulsar-client-all? Can we just shade it? @lhotari

@dao-jun You can always try. :) I don't think that shading is a solution. The reason is that it's expected to work with protoc generated classes which extend com.google.protobuf.GeneratedMessageV3. If you shade protobuf, that would break.

lhotari commented 5 months ago

slightly related to #19565 since that's making improvements in protobuf support.

Shawyeok commented 5 months ago

From 3.0.0, com.google.protobuf:protobuf-java is included directly in pulsar-client-admin without relocation.

In my understanding, pulsar-client-admin is a java wrapper for pulsar RESTful api. I'm curious what does protobuf-java in pulsar-client-admin used for?

~ download() {
  version=$1
  wget https://repo1.maven.org/maven2/org/apache/pulsar/pulsar-client-admin/${version}/pulsar-client-admin-${version}.jar
  unzip -tl pulsar-client-admin-${version}.jar | awk 'gsub(/\//,"/") < 3'
}

~ download 3.0.0
--2024-04-06 12:31:10--  https://repo1.maven.org/maven2/org/apache/pulsar/pulsar-client-admin/3.0.0/pulsar-client-admin-3.0.0.jar
Connecting to 127.0.0.1:6152... connected.
Proxy request sent, awaiting response... 200 OK
Length: 25830684 (25M) [application/java-archive]
Saving to: ‘pulsar-client-admin-3.0.0.jar’

pulsar-client-admin-3.0.0.jar                           100%[=============================================================================================================================>]  24.63M  7.83MB/s    in 3.3s

2024-04-06 12:31:15 (7.38 MB/s) - ‘pulsar-client-admin-3.0.0.jar’ saved [25830684/25830684]

Archive:  pulsar-client-admin-3.0.0.jar
    testing: META-INF/                OK
    testing: META-INF/MANIFEST.MF     OK
    testing: META-INF/DEPENDENCIES    OK
    testing: META-INF/LICENSE         OK
    testing: META-INF/NOTICE          OK
    testing: org/                     OK
    testing: org/apache/              OK
    testing: META-INF/maven/          OK
    testing: findbugsExclude.xml      OK
    testing: io/                      OK
    testing: io/airlift/              OK
    testing: META-INF/services/       OK
    testing: META-INF/versions/       OK
    testing: META-INF/io.netty.versions.properties   OK
    testing: META-INF/native/         OK
    testing: META-INF/native-image/   OK
    testing: META-INF/native/libnetty_resolver_dns_native_macos_aarch_64.jnilib   OK
    testing: META-INF/native/libnetty_resolver_dns_native_macos_x86_64.jnilib   OK
    testing: META-INF/LICENSE.md      OK
    testing: META-INF/NOTICE.md       OK
    testing: META-INF/NOTICE.markdown   OK
    testing: META-INF/LICENSE.txt     OK
    testing: META-INF/NOTICE.txt      OK
    testing: lib/                     OK
    testing: lib/libcpu-affinity.so   OK
    testing: META-INF/nar/            OK
    testing: META-INF/native/libnetty_transport_native_io_uring_x86_64.so   OK
    testing: META-INF/native/libnetty_transport_native_io_uring_aarch_64.so   OK
    testing: com/                     OK
    testing: com/google/              OK
    testing: google/                  OK
    testing: google/protobuf/         OK
    testing: google/protobuf/any.proto   OK
    testing: google/protobuf/api.proto   OK
    testing: google/protobuf/descriptor.proto   OK
    testing: google/protobuf/duration.proto   OK
    testing: google/protobuf/empty.proto   OK
    testing: google/protobuf/field_mask.proto   OK
    testing: google/protobuf/source_context.proto   OK
    testing: google/protobuf/struct.proto   OK
    testing: google/protobuf/timestamp.proto   OK
    testing: google/protobuf/type.proto   OK
    testing: google/protobuf/wrappers.proto   OK
    testing: META-INF/native/libnetty_transport_native_epoll_x86_64.so   OK
    testing: META-INF/license/        OK
    testing: META-INF/license/LICENSE.aix-netbsd.txt   OK
    testing: META-INF/license/LICENSE.boringssl.txt   OK
    testing: META-INF/license/LICENSE.mvn-wrapper.txt   OK
    testing: META-INF/license/LICENSE.tomcat-native.txt   OK
    testing: META-INF/native/libnetty_tcnative_linux_x86_64.so   OK
    testing: META-INF/native/libnetty_tcnative_linux_aarch_64.so   OK
    testing: META-INF/native/libnetty_tcnative_osx_x86_64.jnilib   OK
    testing: META-INF/native/libnetty_tcnative_osx_aarch_64.jnilib   OK
    testing: META-INF/native/netty_tcnative_windows_x86_64.dll   OK
    testing: lib/libcirce-checksum.so   OK
    testing: com/scurrilous/          OK
    testing: digesterRules.xml        OK
    testing: properties.dtd           OK
    testing: PropertyList-1.0.dtd     OK
    testing: META-INF/services/org.apache.pulsar.shade.com.fasterxml.jackson.databind.Module   OK
    testing: META-INF/services/org.apache.pulsar.shade.org.glassfish.jersey.internal.inject.InjectionManagerFactory   OK
    testing: META-INF/services/org.apache.pulsar.shade.com.fasterxml.jackson.core.ObjectCodec   OK
    testing: META-INF/services/org.apache.pulsar.shade.org.glassfish.hk2.extension.ServiceLocatorGenerator   OK
    testing: META-INF/services/org.apache.pulsar.shade.javax.ws.rs.ext.MessageBodyWriter   OK
    testing: META-INF/services/org.apache.pulsar.shade.org.glassfish.jersey.internal.spi.AutoDiscoverable   OK
    testing: META-INF/services/com.scurrilous.circe.HashProvider   OK
    testing: META-INF/services/org.apache.pulsar.shade.javax.ws.rs.ext.MessageBodyReader   OK
    testing: META-INF/services/org.apache.pulsar.shade.com.fasterxml.jackson.core.JsonFactory   OK
    testing: META-INF/services/reactor.blockhound.integration.BlockHoundIntegration   OK
No errors detected in compressed data of pulsar-client-admin-3.0.0.jar.
dao-jun commented 5 months ago

@Shawyeok Because admin-api depends on client-api, some classes like Auth MessageId are defined in client-api.

Shawyeok commented 5 months ago

@Shawyeok Because admin-api depends on client-api, some classes like Auth MessageId are defined in client-api.

It looks like pulsar-client-admin just depends on it and not using it.

On the client side, the only feature that depends on protobuf at runtime is ProtobufSchema. However, pulsar-client-admin does not provide related capabilities. Therefore, I think pulsar-client-admin can change to declare dependencies on protobuf in the pom.xml, similar to pulsar-client, instead of directly copying protobuf-related classes into the shaded jar.

dao-jun commented 5 months ago

@Shawyeok
you can try to remove client-api from admin-api and build it.