[GitHub] [tinkerpop] FlorianHockmann commented on issue #1077: TINKERPOP-2135 Fix handling of closed idle connections in Gremlin.Net driver

["FlorianHockmann (GitHub)" <gi...@apache.org>]

>  the iterator is not affected by concurrent modifications (unlike concurrent collections)

This seems to depend on the collection actually. For `ConcurrentDictionary` it seems to include modifications, but for `ConcurrentQueue` not ([Source](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentqueue-1.getenumerator?view=netframework-4.7.2#remarks)):

> The enumeration represents a moment-in-time snapshot of the contents of the queue. It does not reflect any updates to the collection after GetEnumerator was called. The enumerator is safe to use concurrently with reads from and writes to the queue.

I understand your reasoning though for a custom copy-on-write list now like the one you linked in the DataStax Cassandra driver.
Your suggestion of replacing the current `SelectUsedConnection` method with such a round-robin mechanism also makes sense to me. We would get the same benefit as with the enqueue/dequeue mechanism but without the disadvantages you mentioned (possibility to get an empty pool and more expensive operations).
You already suggested that in #903, but I wanted to avoid adding too much complexity there at once and the approach in `SelectUsedConnection` seemed a lot simpler to me at that time. But now that we have a first version of request pipelining working, I think that it makes sense to add this now as an incremental improvement.

> If we need to compare the one with least in-flight requests, we should minimize the amount of checks, for example check connections at index and index+1, instead of checking the whole list (O(2) vs O(n))

Just to make sure that I understand your proposal here correctly: You mean that we would just select the next connection via round-robin and then verify whether it has less than the maximum number of connections and continue these two steps until we have a connection that is below the limit, right? That should usually finish in the first iteration I guess.

Thanks for your suggestions here again so far, @jorgebay! I will give this a try and then update this PR.

I'm just not sure about the task scheduler yet. That sounds like a good idea, but it also adds quite some complexity. So it could make sense to postpone that for 3.4.2. Or do you see that as a must-have already for this version?

[ Full content available at: https://github.com/apache/tinkerpop/pull/1077 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org