RaulGracia commented on a change in pull request #2761:
URL: https://github.com/apache/bookkeeper/pull/2761#discussion_r691265956
##########
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 channels, we can set the TCP user timeout.
+ bootstrap.option(EpollChannelOption.TCP_USER_TIMEOUT,
conf.getTcpUserTimeoutMillis());
Review comment:
In all the `ClientConfiguration` timeout properties, there is defined a
default value, so I have followed the same pattern. Note that there are
properties in that class that set a different default as the one coming from
the original library; for example, `CLIENT_CONNECT_TIMEOUT_MILLIS` has a
default value in `ClientConfiguration` (10 seconds) different to the default
value in Netty (30 seconds).
Also, there are documents ([like this proposal in the gRPC
repository](https://github.com/grpc/proposal/blob/master/A18-tcp-user-timeout.md))
that propose setting `TCP_USER_TIMEOUT` value to 20 seconds by default, which
is similar to what we are setting now (10 seconds).
Overall, leaving this property unset may be hindering the benefits of this
PR unless otherwise specified by the client (who may not know about this
property).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
