[
https://issues.apache.org/jira/browse/TINKERPOP-2486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17421645#comment-17421645
]
ASF GitHub Bot commented on TINKERPOP-2486:
-------------------------------------------
divijvaidya commented on pull request #1465:
URL: https://github.com/apache/tinkerpop/pull/1465#issuecomment-929114562
From a backward compatibility angle, I reviewed this CR for the following
cases:
1. User has a value of maxInProcess > maxSimulatenousUsage
This case should not be impacted because connection would (should!) not have
been borrowed and thus in process requests would never be greater than borrowed
requests.
2. User has a value of maxInProcess < maxSimulatenousUsage
Prior to the change in this PR, at line `if (inFlight >= availableInProcess)
{` requests will wait to be sent to the server in case maxSimulatenous value
has been reached but now, all borrowed requests will be sent to the server,
i.e. after this PR, users with the above configuration might observe more
simultaneous requests per connection than earlier. This makes this change risky
for users who have adjusted the configuration after deliberate load testing on
their servers.
I would **vote - 1** for this change to be released in 3.4.x and 3.5.x
because of the potential of unanticipated impact of removing inProcess
configuration for existing users of the client.
I would **vote +1** for this change to be released in 3.6.x / master **after
the comment associated with announceAvailableConnection has been addressed**.
Separately, some additional work also needs to be taken up associated with
this change (out of the scope of this PR):
1. Perf testing the impact of synchronised method introduced in this change
for high throughput workload.
2. Deprecate max/minInprocess configuration
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
> Client does not load balance requests across available connections
> ------------------------------------------------------------------
>
> Key: TINKERPOP-2486
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2486
> Project: TinkerPop
> Issue Type: Bug
> Components: driver
> Affects Versions: 3.4.8, 3.4.9
> Reporter: Divij Vaidya
> Priority: Major
>
> The client does not load balance requests across connections in a threadpool
> which cause a request failing with timeout even when a connection is
> available. To verify this, the following test fails:
> {code:java}
> @Test
> public void shouldBalanceConcurrentRequestsAcrossConnections() throws
> InterruptedException {
> final int connPoolSize = 16;
> final Cluster cluster = TestClientFactory.build()
> .minConnectionPoolSize(connPoolSize)
> .maxConnectionPoolSize(connPoolSize)
> .create();
> final Client.ClusteredClient client = cluster.connect();
> client.init();
> try {
> final RequestMessage.Builder request =
> client.buildMessage(RequestMessage.build(Tokens.OPS_EVAL))
> .add(Tokens.ARGS_GREMLIN, "Thread.sleep(5000)");
> final Callable<Connection> sendQueryCallable = () ->
> client.chooseConnection(request.create());
> final List<Callable<Connection>> listOfTasks = new ArrayList<>();
> for (int i=0; i<connPoolSize; i++) {
> listOfTasks.add(sendQueryCallable);
> }
> Set<String> channels = new HashSet<>();
> final List<Future<Connection>> executorSubmitFutures =
> executorServiceForTesting.invokeAll(listOfTasks);
> executorSubmitFutures.parallelStream().map(fut -> {
> try {
> return fut.get();
> } catch (InterruptedException e) {
> e.printStackTrace();
> } catch (ExecutionException e) {
> e.printStackTrace();
> }
> return null;
> }).forEach(conn -> channels.add(conn.getChannelId()));
>
> System.out.println(channels.size());
> } finally {
> cluster.close();
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
