Fix issue with binary compatibility with older grpc versions at runtime in the client #3997
Conversation
…me in the bookkeeper client. - prevent possible NoSuchMethodError errors about DnsNameResolverProvider.newNameResolver method see grpc/grpc-java@fcb5c54#diff-b04e884de51ed12ff79482f600a2d4ec18e405ee189a4952ae35f4d2742b7160L50
…#2) (#9812) This change has these main aspects to it: 1. Removal of any name resolution responsibility from ManagedChannelImpl 2. Creation of a new RetryScheduler to own generic retry logic - Can also be used outside the name resolution context 3. Creation of a new RetryingNameScheduler that can be used to wrap any polling name resolver to add retry capability 4. A new facility in NameResolver to allow implementations to notify listeners on the success of name resolution attempts - RetryingNameScheduler relies on this
zymap
added
dependencies
…me in the bookkeeper client. (#3997) ### Motivation - grpc version was upgraded to 1.56.0 in #3992 - that breaks binary compatibility for DnsNameResolverProvider class - see grpc/grpc-java@fcb5c54#diff-b04e884de51ed12ff79482f600a2d4ec18e405ee189a4952ae35f4d2742b7160L50 ### Changes - make the field type NameResolverProvider instead of DnsNameResolverProvider - this prevents possible NoSuchMethodError errors about DnsNameResolverProvider.newNameResolver method (cherry picked from commit ffc8e8b)
|
This PR will also be useful to cherry-pick on maintenance branches to support grpc-java >= 1.54.0 |
|
In Pulsar we need Bookkeeper 4.16.2 release with this fix until we can make the switch to grpc-java 1.56 on Pulsar side. @zymap @hangc0276 @eolivelli @nicoloboschi @dlg99 Any timeline for 4.16.2 release? |
|
@lhotari This PR is already included in the 4.16.2. I am preparing the release, and I think we can release it in the next week. But I met some failures in Pulsar's CI and need to take a look before sending the vote. It looks like something is wrong with the bookie that causes the pulsar's tests to fail. |
|
It seems I met the same issue mentioned here protocolbuffers/protobuf#11986 Pulsar errors: Caused by: java.lang.IllegalAccessError: class org.apache.bookkeeper.proto.DataFormats$BookieServiceInfoFormat$Endpoint tried to access method 'com.google.protobuf.LazyStringArrayList com.google.protobuf.LazyStringArrayList.emptyList()' (org.apache.bookkeeper.proto.DataFormats$BookieServiceInfoFormat$Endpoint and com.google.protobuf.LazyStringArrayList are in unnamed module of loader 'app') |
@zymap I wonder what the resolution is? Perhaps there's an option to downgrade protobuf to 3.21.x ? |
|
@zymap Perhaps setting It seems to be designed that way that code gen should be generated with the oldest version of the supported library. |
@lhotari I found the reason for this error. It is caused by the protobuf-java version. In the generated code, it uses In the pulsar side, we are using 3.19.6, which the They have exposed the method to the public since 3.22.0 protocolbuffers/protobuf@c658e27 This means if we want to change upgrade protobuf to 3.22.0+, we must require all the sub-project, which depends on the bookkeeper to upgrade protobuf to 3.22.0+. It should not be acceptable in a minor release. So I will exclude this PR from the 4.16.2 release and include it in the next major release. Downgrade to 3.21.x seems everything will work well. And the CVE can be fixed by a lower version of grpc(1.54+), we can use an old grpc version. |
--- ### Motivation We upgrade grpc and protobuf to address CVE-2023-32732. But it requires the protobuf 3.22+. In protobuf 3.22.0, it introduces a breaking change. It require all the sub-project, which depends on the bookkeeper to upgrade protobuf to 3.22.0+. It should not be acceptable in a minor release. So we use a lower version of grpc and protobuf to address the CVE issue. See more context: apache#3997
…ng change (#4001) * Downgrade grpc to 1.54.1 --- ### Motivation We upgrade grpc and protobuf to address CVE-2023-32732. But it requires the protobuf 3.22+. In protobuf 3.22.0, it introduces a breaking change. It require all the sub-project, which depends on the bookkeeper to upgrade protobuf to 3.22.0+. It should not be acceptable in a minor release. So we use a lower version of grpc and protobuf to address the CVE issue. See more context: #3997
…me in the bookkeeper client. (#3997) ### Motivation - grpc version was upgraded to 1.56.0 in #3992 - that breaks binary compatibility for DnsNameResolverProvider class - see grpc/grpc-java@fcb5c54#diff-b04e884de51ed12ff79482f600a2d4ec18e405ee189a4952ae35f4d2742b7160L50 ### Changes - make the field type NameResolverProvider instead of DnsNameResolverProvider - this prevents possible NoSuchMethodError errors about DnsNameResolverProvider.newNameResolver method (cherry picked from commit ffc8e8b)
…ng change (#4001) * Downgrade grpc to 1.54.1 --- ### Motivation We upgrade grpc and protobuf to address CVE-2023-32732. But it requires the protobuf 3.22+. In protobuf 3.22.0, it introduces a breaking change. It require all the sub-project, which depends on the bookkeeper to upgrade protobuf to 3.22.0+. It should not be acceptable in a minor release. So we use a lower version of grpc and protobuf to address the CVE issue. See more context: #3997 (cherry picked from commit 9f63cf7)
…me in the bookkeeper client. (apache#3997) ### Motivation - grpc version was upgraded to 1.56.0 in apache#3992 - that breaks binary compatibility for DnsNameResolverProvider class - see grpc/grpc-java@fcb5c54#diff-b04e884de51ed12ff79482f600a2d4ec18e405ee189a4952ae35f4d2742b7160L50 ### Changes - make the field type NameResolverProvider instead of DnsNameResolverProvider - this prevents possible NoSuchMethodError errors about DnsNameResolverProvider.newNameResolver method
Motivation
Changes