[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2761: Issue 2482: Added TCP_USER_TIMEOUT to Epoll channel config

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

eolivelli commented on a change in pull request #2761:
URL: https://github.com/apache/bookkeeper/pull/2761#discussion_r687636384



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
##########
@@ -540,6 +541,8 @@ protected ChannelFuture connect() {
         bootstrap.group(eventLoopGroup);
         if (eventLoopGroup instanceof EpollEventLoopGroup) {
             bootstrap.channel(EpollSocketChannel.class);
+            // For Epoll, also set the connection timeout via TCP_USER_TIMEOUT.
+            bootstrap.option(EpollChannelOption.TCP_USER_TIMEOUT, conf.getClientConnectTimeoutMillis());

Review comment:
       I agree that it is better to add a new configuration parameter.
   
   also this option is not like `clientConnectTimeoutMillis`, that usually is meant to give a maximum time for the server to set up the connection, I see it more like `readTimeout` (https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java#L586) that is a generic timeout for reads from network (after the connection is properly set up)




-- 
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: issues-unsubscribe@bookkeeper.apache.org

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