kenhuuu commented on code in PR #1833:
URL: https://github.com/apache/tinkerpop/pull/1833#discussion_r1038716577
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java:
##########
@@ -123,7 +124,9 @@ protected void initChannel(final SocketChannel
socketChannel) throws Exception {
}
if (sslCtx.isPresent()) {
-
pipeline.addLast(sslCtx.get().newHandler(socketChannel.alloc(),
connection.getUri().getHost(), connection.getUri().getPort()));
+ SslHandler sslHandler =
sslCtx.get().newHandler(socketChannel.alloc(), connection.getUri().getHost(),
connection.getUri().getPort());
+ sslHandler.setHandshakeTimeoutMillis(0);
Review Comment:
That "SSLEngine closed already" exception is just a general error message
because the handshake future wasn't given a specific cause. When updating the
handshake future with an SSLHandshakeException, the stack trace looks like
`[ERROR] org.apache.tinkerpop.gremlin.driver.Handler$GremlinResponseHandler
- Could not process the response
javax.net.ssl.SSLHandshakeException: SSL handshake timed out.
at
org.apache.tinkerpop.gremlin.driver.handler.WebSocketClientHandler.userEventTriggered(WebSocketClientHandler.java:127)
at
io.netty.channel.AbstractChannelHandlerContext.invokeUserEventTriggered(AbstractChannelHandlerContext.java:346)
at
io.netty.channel.AbstractChannelHandlerContext.invokeUserEventTriggered(AbstractChannelHandlerContext.java:332)
at
io.netty.channel.AbstractChannelHandlerContext.fireUserEventTriggered(AbstractChannelHandlerContext.java:324)
at
io.netty.handler.codec.http.websocketx.WebSocketClientProtocolHandshakeHandler$2.run(WebSocketClientProtocolHandshakeHandler.java:121)
at io.netty.util.concurrent.PromiseTask.runTask(PromiseTask.java:98)
at
io.netty.util.concurrent.ScheduledFutureTask.run(ScheduledFutureTask.java:170)
at
io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
at
io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167)
at
io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:503)
at
io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:995)
at
io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
at java.base/java.lang.Thread.run(Thread.java:829)`
For more information, this is what the 3.5.4 driver logs show:
`[ERROR] org.apache.tinkerpop.gremlin.driver.Handler$GremlinResponseHandler
- Could not process the response
io.netty.handler.ssl.SslHandshakeTimeoutException: handshake timed out after
10000ms
at io.netty.handler.ssl.SslHandler$7.run(SslHandler.java:2113)
at io.netty.util.concurrent.PromiseTask.runTask(PromiseTask.java:98)
at
io.netty.util.concurrent.ScheduledFutureTask.run(ScheduledFutureTask.java:170)
at
io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
at
io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167)
at
io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:503)
at
io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:995)
at
io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
at java.base/java.lang.Thread.run(Thread.java:829)`
If you think this new stack trace is fine then we can keep the default SSL
handshake timeout as 0. Otherwise, it may be cleaner to just use a percentage
of connectionSetupTimeoutMillis. And just to be clear, this is just an
exception that is logged in logs. The user won't see this exception as it is
part of the close/cleanup process happening in the background.
--
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]