Github user FlorianHockmann commented on the issue:
https://github.com/apache/tinkerpop/pull/903
Ok, I think I finally got your proposition. Took me way longer than it
should have to be honest ð
The changes would basically look like this if I got everything right:
* `Connection`:
* Gets its own write queue
* New requests are simply appended to this queue.
* Sends messages from its queue as fast as possible, one `SendAsync()`
after another
* Maintains a lookup table: `requestId` --> response callback
* Continuously `awaits` `ReceiveAsync()` and invokes the right callback
for every received response based on the `requestId`
* `ConnectionPool`:
* Keeps all created `Connections` in its pool (they aren't taken out of
the pool completely while in use any more)
* Either returns a random `Connection` or returns the least-used
`Connection` based on an in-flight counter on the `Connection` (the latter
would be the better solution but is a bit more complex).
Is that the design you are proposing or did I still get something wrong?
I guess what kept me the entire time from understanding this is that I for
some reason assumed that messages could be received on a different WebSocket
connection than where the send was performed. That would have allowed for an
easier approach without the write queue.
This brings us now back to your initial question of whether we want to
merge the changes from this PR first or whether we want to implement it
together with the request pipelining we now discussed in great length.
@spmallette: Could you say something about the planned release date for
TinkerPop 3.4.0? The last statement I remember was something like _end of
summer_.
I could probably try to implement the request pipelining in October, but
when code freeze is before that or in the beginning of October, then I'd say
that we should definitely merge this PR first. Otherwise, it would have to wait
for TinkerPop 3.5.0.
Your (@jorgebay) argument for implementing both tickets together was:
> I think we should take the approach in B, otherwise implementing it
without thinking of TINKERPOP-1775 might introduce unnecessary complexity (i.e:
a queue of connections that get enqueued and dequeued for each request).
but this enqueueing and dequeueing of connections is already present in the
current implementation.
This PR basically adds these points:
* Min and max sizes
* [CAS pattern](https://en.wikipedia.org/wiki/Compare-and-swap) to ensure
that the max size is respected
* An `AsyncAutoResetEvent` to replace the synchronous lock
I think those three aspects would still be needed with request pipelining.
So, in my opinion, this PR is really an incremental improvement that doesn't
add much complexity that we might need to remove again in order to implement
request pipelining.
---
