FlorianHockmann commented on issue #1077: Tinkerpop 2135 Fix handling of closed idle connections in Gremlin.Net driver URL: https://github.com/apache/tinkerpop/pull/1077#issuecomment-469659360 Thanks for your review, @jorgebay! I'm not sure if you're following on [TINKERPOP-2148](https://issues.apache.org/jira/browse/TINKERPOP-2148), but user Zaoshi suggested there to switch from `ConcurrentBag` to `ConcurrentQueue`. This would have two nice advantages for us right now: 1. We don't have to iterate over all connections and find the least used one. Instead, we can simply dequeue/enqueue connections and use the first that hasn't reached it's limit. This is effectively a round-robin mechanism which is probably more efficient than our current approach. 2. Removal of a closed connection would be as simple as not enqueueing it again during this process. Would be good to hear your opinion on that. What's the advantage of such a custom copy-on-write list? Only avoiding the expensive `Count` implementation of `ConcurrentDictionary`? That seems to be at least less expensive for [`ConcurrentQueue`](https://github.com/Microsoft/referencesource/blob/3b1eaf5203992df69de44c783a3eda37d3d4cd10/mscorlib/system/collections/Concurrent/ConcurrentQueue.cs#L394). But we would probably have to maintain our own connection counter if we moved to such a dequeue/enqueue approach as that would change the count all the time. In general, I would prefer if we could stick with the default collection types if possible and only add our own types if we really have to. > To ensure that concurrent modifications to the pool don't occur, e.g., pool growing and closing, we could use something like a ordered task scheduler. I also thought about how we could prevent that, but I wanted to postpone it to a subsequent PR. A task scheduler sounds like a good solution in general. Do you know whether the default [TaskScheduler](https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.taskscheduler) would work here? I haven't worked with that yet myself. With a task scheduler we would probably only have to change the way we replace dead connections. Currently, all connections are removed from the pool if a request fails due to a network error and new connections are created the next time a connection is needed. That would not work anymore if a scheduled task is used to create new connections as we probably can't really wait for that (?)
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
