[
https://issues.apache.org/jira/browse/TINKERPOP-2814?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17634829#comment-17634829
]
ASF GitHub Bot commented on TINKERPOP-2814:
-------------------------------------------
divijvaidya commented on code in PR #1833:
URL: https://github.com/apache/tinkerpop/pull/1833#discussion_r1023965902
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Cluster.java:
##########
@@ -205,6 +205,7 @@ private static Builder getBuilderFromSettings(final
Settings settings) {
.maxConnectionPoolSize(settings.connectionPool.maxSize)
.minConnectionPoolSize(settings.connectionPool.minSize)
.connectionSetupTimeoutMillis(settings.connectionPool.connectionSetupTimeoutMillis)
+
.sslHandshakeTimeoutMillis(settings.connectionPool.sslHandshakeTimeoutMillis)
Review Comment:
Hey @kenhuuu
Thank you for your patience so far. I am trying to explore the possibility
of having only one timeout that the customer has to manage i.e. the connection
timeout. As you rightly pointed out, we need to ensure that our implementation
will not lose details of where the timeout occurred.
The `SslHandler` class in Netty has public methods which allow to check
whether the ssl handshake has been completed. For example, one can use
`SslHandler#handshakeFuture.isComplete()` to check whether the handshake has
been completed or not. Alternatively one can listen to
`SslHandshakeCompletionEvent` as mentioned in
https://netty.io/4.1/api/io/netty/handler/ssl/SslHandler.html
> Beside using the handshake
[ChannelFuture](https://netty.io/4.1/api/io/netty/channel/ChannelFuture.html)
to get notified about the completion of the handshake it's also possible to
detect it by implement the
[ChannelInboundHandler.userEventTriggered(ChannelHandlerContext,
Object)](https://netty.io/4.1/api/io/netty/channel/ChannelInboundHandler.html#userEventTriggered-io.netty.channel.ChannelHandlerContext-java.lang.Object-)
method and check for a
[SslHandshakeCompletionEvent](https://netty.io/4.1/api/io/netty/handler/ssl/SslHandshakeCompletionEvent.html).
Hence, we could potentially do an implementation such as the following in
our client:
1. Set SslHandler#setHandshakeTimeout to 0 [which means that handshake is
not bound by the
timeout](https://github.com/netty/netty/blob/4.1/handler/src/main/java/io/netty/handler/ssl/SslHandler.java#L2105).
2. Set a timeout on overall connection setup (which we already do).
3. When timeout hits for overall connection setup, check what stage is the
connection setup at? Is the ssl handshake complete (can be checked using the
future as as mentioned above)? If not, then cancel it. Is the websocket
handshake complete? Is the SASL handshake complete?
4. Depending on the stage where the overall connection is at, we can
generate an appropriate error message to the application.
What do you think about this approach?
> Add a SSL handshake timeout configuration to the driver
> -------------------------------------------------------
>
> Key: TINKERPOP-2814
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2814
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
> Affects Versions: 3.5.4
> Reporter: Stephen Mallette
> Priority: Blocker
>
> The java driver currently relies on the default 10 second SSL handshake
> timeout defined by Netty. Add a configuration to the driver to allow users to
> change that setting.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
