[GitHub] [pulsar] eolivelli commented on a change in pull request #12917: Protocol Handlers and Proxy Extensions: Fix bootstrap and add tests

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #12917:
URL: https://github.com/apache/pulsar/pull/12917#discussion_r758185401



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -385,16 +385,25 @@ public void startProtocolHandlers(
     private void startProtocolHandler(String protocol,
                                       SocketAddress address,
                                       ChannelInitializer<SocketChannel> initializer) throws IOException {
-        ServerBootstrap bootstrap = defaultServerBootstrap.clone();
+
         ServiceConfiguration configuration = pulsar.getConfiguration();
         boolean useSeparateThreadPool = configuration.isUseSeparateThreadPoolForProtocolHandlers();
+        ServerBootstrap bootstrap;
         if (useSeparateThreadPool) {
+            bootstrap = new ServerBootstrap();
+            bootstrap.childOption(ChannelOption.ALLOCATOR, PulsarByteBufAllocator.DEFAULT);
+            bootstrap.childOption(ChannelOption.TCP_NODELAY, true);
+            bootstrap.childOption(ChannelOption.RCVBUF_ALLOCATOR,
+                    new AdaptiveRecvByteBufAllocator(1024, 16 * 1024, 1 * 1024 * 1024));
+            EventLoopUtil.enableTriggeredMode(bootstrap);

Review comment:
       I tough about it.
   But if in the future we want to change the configuration for the Pulsar Endpoint bootstrap, it is very likely that we would change the new method that you suggest.
   This will affect Protocol Handlers without notice.
   
   So having two separate methods for the PH and for the Pulsar endpoints is better
   
   I am also not sure that we want "EventLoopUtil.enableTriggeredMode(bootstrap);" for the PHs
   it is something we like for Pulsar probably but it is a very advanced feature.
   
   Ideally it would be better that the PH is free to bootstrap Netty stuff and just register itself in the Broker in order to let the Broker drive the lifecycle.
   For instance sometimes you wouldn't like to use Netty, for the KOP Schema Registry I had to use Netty but it would have been far easier to use Jetty.
   but this deserves a separate discussion




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org