[
https://issues.apache.org/jira/browse/TINKERPOP-2369?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17181206#comment-17181206
]
ASF GitHub Bot commented on TINKERPOP-2369:
-------------------------------------------
spmallette commented on pull request #1309:
URL: https://github.com/apache/tinkerpop/pull/1309#issuecomment-677678867
Thanks for this - a few minor comments/nits:
1. Could you please rename tests that start with "Test*" to our more
standard "should*"
2. Have a look at where you might add more `final` declarations to match the
code style.
3. I like the idea of integration tests with the `SimpleWebSocketServer` -
very smart
4. I'm surprised you found as many places as you did where `Cluster.close()`
wasn't called.
5. I don't see where the semantics of any of the
`GremlinDriverIntegrateTest` tests changed so it seems you accomplished this
fix without breaking behavioral changes in the driver...that's nice.
6. Maybe I missed it but was there a test in Gremlin Server to validate any
of this change - perhaps it was already better tested by way of your
`SimpleWebSocketServer` tests?
7. I assume you will polish up the commit history a bit on merge and squash
things down to a few (or one) commits?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Connections in ConnectionPool are not replaced in background when underlying
> channel is closed
> ----------------------------------------------------------------------------------------------
>
> Key: TINKERPOP-2369
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2369
> Project: TinkerPop
> Issue Type: Bug
> Components: driver
> Affects Versions: 3.4.1
> Reporter: Johannes Carlsen
> Priority: Major
>
> Hi Tinkerpop team!
>
> We are using the Gremlin Java Driver to connect to an Amazon Neptune cluster.
> We are using the IAM authentication feature provided by Neptune, which means
> that individual websocket connections are closed by the server every 36
> hours, when their credentials expire. The current implementation of the
> driver does not handle this situation well, as the Connection whose channel
> has been closed by the server remains in the ConnectionPool. The connection
> is only reported as dead and replaced when when it is later chosen by the
> LoadBalancingStrategy to server a client request, which inevitably fails when
> the connection attempts to write to the closed channel.
> A fix for this bug would cause the connection pool to be automatically
> refreshed in the background by either the keep-alive mechanism, which should
> replace a connection if a keep-alive request fails, or by adding a listener
> for the close frame being sent to the underlying channel to replace the
> connection. Without a fix, the only way to recover from a stale connection is
> to retry the request at the cluster level, which will allow the request to be
> directed to a different connection.
> I noticed a PR out for the .NET client to fix this behavior:
> [https://github.com/apache/tinkerpop/pull/1279.] We are hoping for something
> similar in the Gremlin Java Driver.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
