>  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 [email protected]

Reply via email to