-
Notifications
You must be signed in to change notification settings - Fork 26
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
TACO-326-supreme removing the idle channel handler from the ClientCha… #260
Conversation
…nnelInitializer so that we don't manually close the channel just because it is idling, we are trying to keep the client channels open so when we reuse the client we don't have to do the whole handshake over again to connect
@@ -64,7 +63,6 @@ protected void initChannel(Channel channel) throws Exception { | |||
} | |||
channel | |||
.pipeline() | |||
.addLast("idle handler", new XioIdleDisconnectHandler(60, 60, 60)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think client is meant to be used not just as a proxy client... i wonder if the disconnect handler should be removed by the proxy client factory instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... or better yet a configurable timeout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah making update, good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add tests as well
…ting the base reference.conf to fill this in for us, next adding test to ClientChannelInitializerTest.java
private final Map<ChannelOption<Object>, Object> bootstrapOptions; | ||
private final String name; | ||
private final TlsConfig tls; | ||
private final boolean messageLoggerEnabled; | ||
private final InetSocketAddress local; | ||
private final InetSocketAddress remote; | ||
private final IdleTimeoutConfig idleTimeoutConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like!
@@ -163,6 +163,8 @@ xio { | |||
remotePort = 0 | |||
localIp = "" | |||
localPort = 0 | |||
idleTimeoutEnabled = false | |||
idleTimeoutDuration = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point we may want the type of incoming connection to determine if there’s no idle timeout. This is fine for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k
…nnelInitializer so that we don't manually close the channel just because it is idling, we are trying to keep the client channels open so when we reuse the client we don't have to do the whole handshake over again to connect