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

[improve][misc] Upgrade to Netty 4.1.111.Final and switch to use grpc-netty-shaded #22892

Merged
merged 11 commits into from
Jun 12, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Jun 11, 2024

Fixes #22601
Fixes #21892
Fixes #19460

Motivation

Netty 4.1.111.Final contains important fixes:

These address Broker stability when TLS with Bookkeeper V2 protocol is used between Broker and Bookies.
This will also allow removing the extra buffer copies that have been in place in Pulsar broker and Pulsar client when TLS is enabled.

public static class CopyingEncoder extends ChannelOutboundHandlerAdapter {
@Override
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception {
if (msg instanceof ByteBufPair) {
ByteBufPair b = (ByteBufPair) msg;
// Some handlers in the pipeline will modify the bytebufs passed in to them (i.e. SslHandler).
// For these handlers, we need to pass a copy of the buffers as the source buffers may be cached
// for multiple requests.
try {
ctx.write(b.getFirst().copy(), ctx.voidPromise());
ctx.write(b.getSecond().copy(), promise);
} finally {
ReferenceCountUtil.safeRelease(b);
}
} else {
ctx.write(msg, promise);
}
}
}

This will be removed in a separate PR.

Modifications

  • Upgrade Netty to 4.1.111.Final version

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari
Copy link
Member Author

lhotari commented Jun 12, 2024

etcd tests break with Netty 4.1.111.Final. There's some compatibility issue.
I also tried upgrading jetcd to 0.8.2 and grpc to 1.64.0, but that didn't help.

@lhotari
Copy link
Member Author

lhotari commented Jun 12, 2024

It seems that grpc-netty dependency will have to be excluded and replaced by grpc-netty-shaded since grpc is compatible only with specific Netty versions.

@lhotari
Copy link
Member Author

lhotari commented Jun 12, 2024

Unfortunately jetcd-core isn't compatible with grpc-netty-shaded. 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. The only solution seems to be to do custom shading for jetcd-core and jetcd-test.

@lhotari
Copy link
Member Author

lhotari commented Jun 12, 2024

I found a reasonable solution by shading jetcd-core in a way where vertx is relocated and included, but grpc is switched to use grpc-netty-shaded instead of grpc-netty. stream-storage-java-client supports switching to use grpc-netty-shaded without making changes to the module.
This resolves the issue with grpc-netty and Netty 4.1.111.Final.

@lhotari lhotari changed the title [improve][misc] Upgrade to Netty 4.1.111.Final [improve][misc] Upgrade to Netty 4.1.111.Final and switch to use grpc-netty-shaded Jun 12, 2024
@merlimat merlimat merged commit 75d7e55 into apache:master Jun 12, 2024
49 of 53 checks passed
merlimat pushed a commit that referenced this pull request Jun 12, 2024
@merlimat
Copy link
Contributor

@lhotari There are quite a few merge conflict everywhere (except for 3.3). I'm not sure how difficult it would be to backport.

@lhotari
Copy link
Member Author

lhotari commented Jun 13, 2024

@lhotari There are quite a few merge conflict everywhere (except for 3.3). I'm not sure how difficult it would be to backport.

I'll handle the backporting. I think it's doable.

lhotari added a commit that referenced this pull request Jun 13, 2024
lhotari added a commit that referenced this pull request Jun 13, 2024
…-netty-shaded (#22892)

(cherry picked from commit 75d7e55)
(cherry picked from commit a982d7b)

# Conflicts:
#	distribution/server/pom.xml
lhotari added a commit that referenced this pull request Jun 13, 2024
lhotari added a commit that referenced this pull request Jun 13, 2024
- version in jetcd-core-shaded/pom.xml needs to match project version
lhotari added a commit to lhotari/pulsar that referenced this pull request Jun 14, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 25, 2024
…-netty-shaded (apache#22892)

(cherry picked from commit 75d7e55)
(cherry picked from commit a982d7b)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 27, 2024
…-netty-shaded (apache#22892)

(cherry picked from commit 75d7e55)
(cherry picked from commit a982d7b)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 27, 2024
…-netty-shaded (apache#22892)

(cherry picked from commit 75d7e55)
(cherry picked from commit a982d7b)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 27, 2024
…-netty-shaded (apache#22892)

(cherry picked from commit 75d7e55)
(cherry picked from commit a982d7b)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 1, 2024
…-netty-shaded (apache#22892)

(cherry picked from commit 75d7e55)
(cherry picked from commit a982d7b)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 1, 2024
…-netty-shaded (apache#22892)

(cherry picked from commit 75d7e55)
(cherry picked from commit a982d7b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants