[GitHub] [tinkerpop] FlorianHockmann commented on pull request #1404: TINKERPOP-2531: Make sure connection pool is restored and brought back to health if server is temporarily unavailable.

[GitBox <gi...@apache.org>]

FlorianHockmann commented on pull request #1404:
URL: https://github.com/apache/tinkerpop/pull/1404#issuecomment-797394617


   Hey @radu-iviniciu and thanks for your contribution to TinkerPop!
   
   The functionality to replace closed connections automatically in the background was only added recently (see [TINKERPOP-2288](https://issues.apache.org/jira/browse/TINKERPOP-2288)). So, there are definitely cases that aren't covered yet by it. The problem you describe in TINKERPOP-2531 is of course a case where the current retry logic isn't enough when the server is offline longer than the retry period as we then don't start another repair attempt. We originally thought about adding a scheduled repair job that checks the pool every minute or so and tries to repair it if necessary but didn't implement it yet because it would add a lot of complexity to the driver that I would like to avoid having if we don't really need it.
   
   I really like your solution to this problem as it's much simpler than a scheduled repair but it still solves the problem. It ensures that we will try again to repair the pool if a new request should be sent but the previous attempt failed and it shouldn't add unnecessary costs as we already have a mechanism to only perform one repair operation in parallel.
   
   I see only one thing missing in this PR: Could you please add a test case for the problem this is trying to solve? We already have a few [`ConnectionPoolTests`](https://github.com/apache/tinkerpop/blob/3.4-dev/gremlin-dotnet/test/Gremlin.Net.UnitTest/Driver/ConnectionPoolTests.cs) that use mocks to create the scenarios they want to test. If you have problems with creating a test case for this, then I can also give it a try.


----------------------------------------------------------------
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:
users@infra.apache.org