[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16166503#comment-16166503 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/704 I just cherry-picked the two commits from @jorgebay's PR and merged tp32 into master afterwards. So master should be in a good state now. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: dotnet >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > Fix For: 3.2.7, 3.3.1 > > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16166463#comment-16166463 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/704 yeah - i remember that now! i wonder how i messed that up. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: dotnet >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16166439#comment-16166439 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user jorgebay commented on the issue: https://github.com/apache/tinkerpop/pull/704 It's weird because 6a3fc39a912bec98707dd69d461955330627df10 was merged by @spmallette on master in 75502eef222b0dfc7aa83c2456ab319a9f9dd1a6 (he beat me to it because I was taking too long to do it) and this branch started in 98e4e1b8201f50e1f34458a82c20f2737aa6ae53, which is down the line. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: dotnet >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16166400#comment-16166400 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/704 what the heck?! I wonder what got messed up there. i don't see a merge from jorge from tp32 to master after that, so i guess it never got in there, yet how could it have been ignored? we did tons of merges from tp32 to master after his merge to tp32. i guess you should cherry-pick them into master and then do your merge. that's really strange. i'm not sure what to make of that. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: dotnet >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16166366#comment-16166366 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/704 Thanks for the explanation @spmallette. I was about to ask about the approach for merging pull requests as the documentation only talks about the case where separate pull requests are used for the different release branches. (Maybe we should extend that for simpler cases where only a pull request to tp32 is enough?) I just tried to merge tp32 to master, but ran into a merge conflict as apparently the changes from TINKERPOP-1744 (PR #690) are missing in master although GitHub shows the merge commit also for the master branch: https://github.com/apache/tinkerpop/commit/6a3fc39a912bec98707dd69d461955330627df10 Do I now have to cherry-pick them to get them into master before I merge tp32 or what is the best approach in cases like this? I'll close the JIRA ticket as soon as the changes are also in master. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: dotnet >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16166122#comment-16166122 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/704 @FlorianHockmann When you merge a PR to tp32, please make sure you also merge tp32 to master. Technically it is best to do that locally first, make sure both tp32 and master are building properly, then push master and then tp32. Also, the JIRA for this PR still needs to closed. Thanks. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: dotnet >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16166052#comment-16166052 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/704 > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: dotnet >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16164670#comment-16164670 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/704 VOTE +1 > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: dotnet >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16160953#comment-16160953 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user jorgebay commented on the issue: https://github.com/apache/tinkerpop/pull/704 I've filed the tickets 1774 and 1775 for the pool enhancements. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16158731#comment-16158731 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/704 > Should we file a different ticket for the pool improvements? Yes, I think a dedicated ticket for those improvements is a good idea and it makes probably sense if you create it as you already made some good suggestions. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16158424#comment-16158424 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user jorgebay commented on the issue: https://github.com/apache/tinkerpop/pull/704 About request pipelining, you can send in a queue one by one regardless of whether or not it was received. On the read side, once a message is received you issue a following call to `ReceiveAsync()` (regardless whether a message has been sent). That way the client can send x messages without waiting for a response. Should we file a different ticket for the pool improvements? About the changes in this pr: VOTE +1 > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16155878#comment-16155878 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/704 I just pushed a commit that addresses your comments, @jorgebay. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16155170#comment-16155170 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/704 Thanks for your comments so far, I'll implement the changes you suggested. I definitely agree that the driver and especially the connection pool needs to be improved and I would really appreciate any effort you (and of course als anyone else) put into this as I'm not exactly an expert in writing database drivers. Could you clarify what you mean with _request pipelining on the same connection_? If you mean sending multiple requests in parallel over the same websocket connection, then I think that it's not possible as [the documentation](https://docs.microsoft.com/en-us/dotnet/api/system.net.websockets.clientwebsocket.sendasync?view=netframework-4.7#Remarks) states that > Exactly one send and one receive is supported on each ClientWebSocket object in parallel. The upside of this is of course that the implementation of the `Connection` and the `ConnectionPool` is much simpler when every `Connection` can just send one single request at a time as we don't have to match incoming responses with the correct requests they belong to. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16155072#comment-16155072 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user jorgebay commented on the issue: https://github.com/apache/tinkerpop/pull/704 I've left some line comments on the pr. Apart from the issue this patch is addressing, we should create separate tickets for issues related to the .NET driver pool: - Blocking (use of `lock`). - An unbounded number of connections can be created under pressure. - Lack of request pipelining on the same connection. I wasn't able to focus on the .NET driver itself, my initial priority was getting a stable `GraphTraversal` API and bytecode serialization, but we can tackle these issues progressively in the next versions. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16155059#comment-16155059 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user jorgebay commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/704#discussion_r137214097 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs --- @@ -74,6 +85,36 @@ private void AddConnection(Connection connection) } } +private void ConsiderUnavailable() +{ +CloseAndRemoveAllConnections(); +} + +private void CloseAndRemoveAllConnections() +{ +lock (_connectionsLock) +{ +TeardownAsync().WaitUnwrap(); +RemoveAllConnections(); +} +} + +private void RemoveAllConnections() +{ +while (!_connections.IsEmpty) +{ +_connections.TryTake(out var connection); +connection.Dispose(); +} +} + +private async Task TeardownAsync() +{ +var closeTasks = new List(_connections.Count); --- End diff -- The list isn't necessary, you can use the `Task.WhenAll(IEnumerable)` overload instead: `Task.WhenAll(_connections.Select(c => c.CloseAsync()))` > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16155054#comment-16155054 ] ASF GitHub Bot commented on TINKERPOP-1766: --- Github user jorgebay commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/704#discussion_r137213383 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs --- @@ -74,6 +85,36 @@ private void AddConnection(Connection connection) } } +private void ConsiderUnavailable() +{ +CloseAndRemoveAllConnections(); +} + +private void CloseAndRemoveAllConnections() +{ +lock (_connectionsLock) +{ +TeardownAsync().WaitUnwrap(); +RemoveAllConnections(); +} +} + +private void RemoveAllConnections() +{ +while (!_connections.IsEmpty) --- End diff -- Using `TryTake()` and `IsEmpty` can create race conditions. Use `while (bag.TryTake(out c))` instead. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1766) Gremlin.Net: Closed connections should not be re-used
[ https://issues.apache.org/jira/browse/TINKERPOP-1766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16151537#comment-16151537 ] ASF GitHub Bot commented on TINKERPOP-1766: --- GitHub user FlorianHockmann opened a pull request: https://github.com/apache/tinkerpop/pull/704 TINKERPOP-1766 Gremlin.Net: Add handling for closed connections https://issues.apache.org/jira/browse/TINKERPOP-1766 This avoids `Connections` from being added back to the `ConnectionPool` when they were closed which happens when the server gets unavailable. In that case all existing `Connections` are removed from the `ConnectionPool` and closed as they are most likely also not `Open` anymore. This is in line with the handling for closed connections in `gremlin-driver`. Unfortunately, I don't know of a good way to test something like this. We probably would have to make internal classes like `Connection` public which isn't that nice as users shouldn't see them in my opinion. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1766 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/704.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #704 commit 98e4e1b8201f50e1f34458a82c20f2737aa6ae53 Author: florianhockmann Date: 2017-09-02T15:54:39Z Add handling for closed connections Closed connections are not returned to the ConnectionPool as the client cannot send any further requests with them. Additionally, the host is considered to be unavailable when a connection was closed and all other connections are closed and removed from the ConnectionPool. This is in line with the handling for closed connection in gremlin-driver. commit d9a4f79f7c568761461dc11d22fbf907578f7d14 Author: florianhockmann Date: 2017-09-02T16:00:52Z Update NuGet packages of test projects One of the dependencies (Castle.Core) produced a warning because one of its dependencies couldn't be found in the specified version. This doesn't occur anymore for the updated version. > Gremlin.Net: Closed connections should not be re-used > -- > > Key: TINKERPOP-1766 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1766 > Project: TinkerPop > Issue Type: Bug > Components: language-variant >Affects Versions: 3.3.0, 3.2.6 >Reporter: Florian Hockmann > > The driver of Gremlin.Net is kept very simle which holds especially true for > the {{ConnectionPool}}. It simply returns every connection to its pool of > usable connections that was {{disposed}} by the client. Unfortunately, this > also applies in case the submit failed due to an already closed connection > which means that the client will get the closed connection back from the > {{ConnectionPool}} later and continues trying to submit messages over this > closed connection. > This can be fixed by checking whether the {{Connection}} is still open before > adding it back to the {{ConnectionPool}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)