[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17017348#comment-17017348
]
Stephen Mallette commented on TINKERPOP-2205:
-
I tried a more advanced test of the {{driver-35}} branch where I installed
Gremlin Server and my {{ProfilingApplication}} referenced above on two separate
systems. With this setup I'm unable to really even execute my tests without the
whole process locking up and I'm not sure what the reasoning is. It does seem
to have something to do with {{DefaultConnectionPool.prepareConnection()}}
blocking indefinitely for some reason, but i'm not clear why that happens at
this point. It could also be that the little test runner i have needs some
additional changes on the {{driver-35}} branch...not sure. Either way, until
this little tool is running nicely and we can more accurately assess the
performance differences between the "old" and "new" I don't think we'll see
this branch merging to {{master}}.
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Assignee: Stephen Mallette
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16943108#comment-16943108
]
Stephen Mallette commented on TINKERPOP-2205:
-
I'm just playing around at the moment. My approach is fairly naive since i'm
just testing locally on my laptop and figured I should get roughly the same
amount of RPS as I was before given the same local setup. I use this utility:
https://github.com/apache/tinkerpop/blob/master/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/util/ProfilingApplication.java
which I've tweaked locally at this point to work the changes on {{driver-35}}.
I expect to push those shortly when my last round of configuration options
completes execution. I usually use this little tool to grab JFRs or to
exercise the driver/server a bit after major changes. It's also only testing
scripts at the moment, though that probably doesn't matter all that much. I
might end up changing that later.
Anyway, i'm seeing {{driver-35}} perform at about 70% of {{master}} in terms of
average rps. So, it's not miserable, but there's a noticeable gap there.
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Assignee: Stephen Mallette
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[ https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16943100#comment-16943100 ] Divij Vaidya commented on TINKERPOP-2205: - Good to hear that we are making progress with the driver. How are you benchmarking the clients to compare the request per second (rps)? I tested with the same configuration of server and client (just change the branch) using the code at [https://github.com/divijvaidya/driver-benchmark-tinkerpop-2205/blob/master/src/main/java/com/divijv/MyBenchmark.java]. Multiple code changes have also been done post the initial benchmark. Let me know if you are not able to achieve the desired performance. At that stage, I will put the driver under a profiler and work towards eliminating bottlenecks. > Use one connection per request for Java client > -- > > Key: TINKERPOP-2205 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2205 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.3.6 >Reporter: Divij Vaidya >Assignee: Stephen Mallette >Priority: Major > Labels: deprecation > > This issue is a tracking item for the conversation in the mailing list > [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E] > which highlights multiple problems and shortcomings in the existing Java > client and proposes a design change in the client connection pooling to > address the same. More specifically, the problems addressed are as follows: > # Difficulty in configuring the client for optimum performance. > # Undocumented dependency of configuration parameters on each other. > # A bad request can impact other requests on the same channel. > # Host is marked as dead even if it is busy serving requests. > # No way to free up server resources if the client has stopped consuming > results. > # No differentiation between retriable and non-retriable exceptions from the > application code. > # Keep alive is only sent when a query is executing, which means that a > connection open for a very long time with no query being sent will be closed > by the server. > # Race condition if the server response reaches before result queue has been > registered. > # Unpredictable behaviour if the server sends an exception followed by a > genuine response for the same request. > # A concurrent hash map (tracking pending requests) is a point of contention > amongst threads. > [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[ https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16943094#comment-16943094 ] ASF GitHub Bot commented on TINKERPOP-2205: --- spmallette commented on pull request #1205: TINKERPOP-2205 Upgrade Netty containing fix for proper close of FixedChannelPool URL: https://github.com/apache/tinkerpop/pull/1205 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: us...@infra.apache.org > Use one connection per request for Java client > -- > > Key: TINKERPOP-2205 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2205 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.3.6 >Reporter: Divij Vaidya >Assignee: Stephen Mallette >Priority: Major > Labels: deprecation > > This issue is a tracking item for the conversation in the mailing list > [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E] > which highlights multiple problems and shortcomings in the existing Java > client and proposes a design change in the client connection pooling to > address the same. More specifically, the problems addressed are as follows: > # Difficulty in configuring the client for optimum performance. > # Undocumented dependency of configuration parameters on each other. > # A bad request can impact other requests on the same channel. > # Host is marked as dead even if it is busy serving requests. > # No way to free up server resources if the client has stopped consuming > results. > # No differentiation between retriable and non-retriable exceptions from the > application code. > # Keep alive is only sent when a query is executing, which means that a > connection open for a very long time with no query being sent will be closed > by the server. > # Race condition if the server response reaches before result queue has been > registered. > # Unpredictable behaviour if the server sends an exception followed by a > genuine response for the same request. > # A concurrent hash map (tracking pending requests) is a point of contention > amongst threads. > [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16943062#comment-16943062
]
Stephen Mallette commented on TINKERPOP-2205:
-
I've been running a utility on {{driver-35}} that tests different driver
configurations. It required some modifications to account for the configuration
changes. I immediately hit two problems:
1. Driver timed-out getting connections or
2. Driver couldn't create a channel (i.e. socket)
The documentation seemed to cover the first issue with upgrading as it
mentioned:
{quote}determine how many requests you anticipate to run in parallel from your
client{quote}
but the following only made it into the PR description:
{quote}A client generating high TPS from a single machine will have to modify
the OS setting for max number of open files, since each connection corresponds
to a single file in linux OS.{quote}
I will add some documentation around that.
Having now formally used {{driver-35}} and really poked about within it, I
think I like that the code relies on more standard netty stuff which is further
supported in:
https://github.com/apache/tinkerpop/pull/1205
I also like the configuration simplicity that was promised in this change.
However, I've not yet been able to achieve the same "request per second" that I
was getting before prior to the changes, but I still may not have the best
configuration to achieve that, so I'm still trying. Another good point now is
that I've passed millions of messages through the driver at this point and it
has behaved in a fairly stable way - there have been no odd errors or
noticeable problems - good sign.
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Assignee: Stephen Mallette
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[ https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16943043#comment-16943043 ] ASF GitHub Bot commented on TINKERPOP-2205: --- divijvaidya commented on pull request #1205: TINKERPOP-2205 Upgrade Netty containing fix for proper close of FixedChannelPool URL: https://github.com/apache/tinkerpop/pull/1205 https://issues.apache.org/jira/browse/TINKERPOP-2205 Upgrade to Netty version which contains the fix for https://github.com/netty/netty/pull/9226. This allows removal of custom implementation TinkerpopFixedChannelPool 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: us...@infra.apache.org > Use one connection per request for Java client > -- > > Key: TINKERPOP-2205 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2205 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.3.6 >Reporter: Divij Vaidya >Assignee: Stephen Mallette >Priority: Major > Labels: deprecation > > This issue is a tracking item for the conversation in the mailing list > [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E] > which highlights multiple problems and shortcomings in the existing Java > client and proposes a design change in the client connection pooling to > address the same. More specifically, the problems addressed are as follows: > # Difficulty in configuring the client for optimum performance. > # Undocumented dependency of configuration parameters on each other. > # A bad request can impact other requests on the same channel. > # Host is marked as dead even if it is busy serving requests. > # No way to free up server resources if the client has stopped consuming > results. > # No differentiation between retriable and non-retriable exceptions from the > application code. > # Keep alive is only sent when a query is executing, which means that a > connection open for a very long time with no query being sent will be closed > by the server. > # Race condition if the server response reaches before result queue has been > registered. > # Unpredictable behaviour if the server sends an exception followed by a > genuine response for the same request. > # A concurrent hash map (tracking pending requests) is a point of contention > amongst threads. > [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16942725#comment-16942725
]
Stephen Mallette commented on TINKERPOP-2205:
-
Picked up this failure on Travis:
{code}
[ERROR] Tests run: 59, Failures: 0, Errors: 1, Skipped: 6, Time elapsed:
451.356 s <<< FAILURE! - in
org.apache.tinkerpop.gremlin.server.GremlinDriverIntegrateTest
[ERROR]
shouldExecuteScriptsInMultipleSession(org.apache.tinkerpop.gremlin.server.GremlinDriverIntegrateTest)
Time elapsed: 9.393 s <<< ERROR!
java.lang.RuntimeException: java.util.concurrent.ExecutionException:
java.lang.RuntimeException:
org.apache.tinkerpop.gremlin.driver.exception.ConnectionException: Timed out
while performing websocket handshake - ensure that client protocol matches
server
at
org.apache.tinkerpop.gremlin.server.GremlinDriverIntegrateTest.shouldExecuteScriptsInMultipleSession(GremlinDriverIntegrateTest.java:1225)
Caused by: java.util.concurrent.ExecutionException: java.lang.RuntimeException:
org.apache.tinkerpop.gremlin.driver.exception.ConnectionException: Timed out
while performing websocket handshake - ensure that client protocol matches
server
at
org.apache.tinkerpop.gremlin.server.GremlinDriverIntegrateTest.shouldExecuteScriptsInMultipleSession(GremlinDriverIntegrateTest.java:1225)
Caused by: java.lang.RuntimeException:
org.apache.tinkerpop.gremlin.driver.exception.ConnectionException: Timed out
while performing websocket handshake - ensure that client protocol matches
server
Caused by: org.apache.tinkerpop.gremlin.driver.exception.ConnectionException:
Timed out while performing websocket handshake - ensure that client protocol
matches server
{code}
Can't say that I see that test as a typically flakey one, so perhaps it's just
a fluke of Travis. Logging it here in case I see it again.
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Assignee: Stephen Mallette
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16941745#comment-16941745
]
Stephen Mallette commented on TINKERPOP-2205:
-
I've had some good docker build runs on {{driver35}} and Travis has had a
couple of clean runs too. I'm not sure if the failing test problem is at issue
anymore for some reasons.
[~divijvaidya] could you please give your ok to the state of the branch now -
here's the diff:
https://github.com/apache/tinkerpop/compare/driver-35#diff-3cb6e28122237b3912bfee9345c869de
If that all looks good to you and assuming my tests keep completing with
success today, I think I'd be comfortable issuing a PR for official review.
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Assignee: Stephen Mallette
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16940916#comment-16940916
]
Stephen Mallette commented on TINKERPOP-2205:
-
I'm not sure what happened, but I tried to rebase {{driver-35}} this morning on
{{master}} and it only had a couple of really easy conflicts to resolve. I
wonder if somehow messed up the rebase when I tried earlier this month. Anyway,
I've force pushed the {{driver-35}} branch and it's now up to date with the
latest stuff on {{master}}
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Assignee: Stephen Mallette
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16928443#comment-16928443
]
stephen mallette commented on TINKERPOP-2205:
-
> Can you also please remind me the specific issues which prevented us from
> merging this PR into master? Is that the flaky tests?
Given my comments on the original PR it seems like the issue was failing tests
as noted by Travis:
https://api.travis-ci.org/v3/job/545093810/log.txt
We also agreed we wanted the newer netty release in play with some of the fixes
you've alluded to - not sure if these were all of them:
https://github.com/netty/netty/pull/9116
https://github.com/netty/netty/pull/9226
Assuming we get a series of good clean builds across a few environments I can't
recall anything else standing in the way of this.
> What is the best way to run integ tests during development right now?
I just do {{mvn clean install && mvn verify -pl gremlin-server
-DskipIntegrationTests=false}} for a general fresh/run build but we'll want to
be sure we get clean {{docker/build.sh -t -i -n}} as well.
Thanks for coming back to this.
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[ https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16927973#comment-16927973 ] Divij Vaidya commented on TINKERPOP-2205: - Yes will do. Can you also please remind me the specific issues which prevented us from merging this PR into master? Is that the flaky tests? What is the best way to run integ tests during development right now? > Use one connection per request for Java client > -- > > Key: TINKERPOP-2205 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2205 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.3.6 >Reporter: Divij Vaidya >Priority: Major > Labels: deprecation > > This issue is a tracking item for the conversation in the mailing list > [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E] > which highlights multiple problems and shortcomings in the existing Java > client and proposes a design change in the client connection pooling to > address the same. More specifically, the problems addressed are as follows: > # Difficulty in configuring the client for optimum performance. > # Undocumented dependency of configuration parameters on each other. > # A bad request can impact other requests on the same channel. > # Host is marked as dead even if it is busy serving requests. > # No way to free up server resources if the client has stopped consuming > results. > # No differentiation between retriable and non-retriable exceptions from the > application code. > # Keep alive is only sent when a query is executing, which means that a > connection open for a very long time with no query being sent will be closed > by the server. > # Race condition if the server response reaches before result queue has been > registered. > # Unpredictable behaviour if the server sends an exception followed by a > genuine response for the same request. > # A concurrent hash map (tracking pending requests) is a point of contention > amongst threads. > [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E -- This message was sent by Atlassian Jira (v8.3.2#803003)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16927625#comment-16927625
]
stephen mallette commented on TINKERPOP-2205:
-
[~divijvaidya] i tried to rebase {{driver-35}} on {{master}} and found it
pretty heavily conflicted. do you have some time to offer a PR that handles
that rebase properly?
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[
https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872693#comment-16872693
]
stephen mallette commented on TINKERPOP-2205:
-
Merged the PR referenced above to the {{driver-25}} branch based on {{master}}
> Use one connection per request for Java client
> --
>
> Key: TINKERPOP-2205
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2205
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
>Affects Versions: 3.3.6
>Reporter: Divij Vaidya
>Priority: Major
> Labels: deprecation
>
> This issue is a tracking item for the conversation in the mailing list
> [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E]
> which highlights multiple problems and shortcomings in the existing Java
> client and proposes a design change in the client connection pooling to
> address the same. More specifically, the problems addressed are as follows:
> # Difficulty in configuring the client for optimum performance.
> # Undocumented dependency of configuration parameters on each other.
> # A bad request can impact other requests on the same channel.
> # Host is marked as dead even if it is busy serving requests.
> # No way to free up server resources if the client has stopped consuming
> results.
> # No differentiation between retriable and non-retriable exceptions from the
> application code.
> # Keep alive is only sent when a query is executing, which means that a
> connection open for a very long time with no query being sent will be closed
> by the server.
> # Race condition if the server response reaches before result queue has been
> registered.
> # Unpredictable behaviour if the server sends an exception followed by a
> genuine response for the same request.
> # A concurrent hash map (tracking pending requests) is a point of contention
> amongst threads.
> [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[ https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872689#comment-16872689 ] ASF GitHub Bot commented on TINKERPOP-2205: --- spmallette commented on pull request #1105: TINKERPOP-2205 Change connection management to single request per channel URL: https://github.com/apache/tinkerpop/pull/1105 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: us...@infra.apache.org > Use one connection per request for Java client > -- > > Key: TINKERPOP-2205 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2205 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.3.6 >Reporter: Divij Vaidya >Priority: Major > Labels: deprecation > > This issue is a tracking item for the conversation in the mailing list > [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E] > which highlights multiple problems and shortcomings in the existing Java > client and proposes a design change in the client connection pooling to > address the same. More specifically, the problems addressed are as follows: > # Difficulty in configuring the client for optimum performance. > # Undocumented dependency of configuration parameters on each other. > # A bad request can impact other requests on the same channel. > # Host is marked as dead even if it is busy serving requests. > # No way to free up server resources if the client has stopped consuming > results. > # No differentiation between retriable and non-retriable exceptions from the > application code. > # Keep alive is only sent when a query is executing, which means that a > connection open for a very long time with no query being sent will be closed > by the server. > # Race condition if the server response reaches before result queue has been > registered. > # Unpredictable behaviour if the server sends an exception followed by a > genuine response for the same request. > # A concurrent hash map (tracking pending requests) is a point of contention > amongst threads. > [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[ https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16852765#comment-16852765 ] Divij Vaidya commented on TINKERPOP-2205: - Sounds good. I will add your tests to the PR and couple of other changes (goodness brought by new Netty version) in the next 2-3 days. > Use one connection per request for Java client > -- > > Key: TINKERPOP-2205 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2205 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.3.6 >Reporter: Divij Vaidya >Priority: Major > > This issue is a tracking item for the conversation in the mailing list > [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E] > which highlights multiple problems and shortcomings in the existing Java > client and proposes a design change in the client connection pooling to > address the same. More specifically, the problems addressed are as follows: > # Difficulty in configuring the client for optimum performance. > # Undocumented dependency of configuration parameters on each other. > # A bad request can impact other requests on the same channel. > # Host is marked as dead even if it is busy serving requests. > # No way to free up server resources if the client has stopped consuming > results. > # No differentiation between retriable and non-retriable exceptions from the > application code. > # Keep alive is only sent when a query is executing, which means that a > connection open for a very long time with no query being sent will be closed > by the server. > # Race condition if the server response reaches before result queue has been > registered. > # Unpredictable behaviour if the server sends an exception followed by a > genuine response for the same request. > # A concurrent hash map (tracking pending requests) is a point of contention > amongst threads. > [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[ https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16851970#comment-16851970 ] stephen mallette commented on TINKERPOP-2205: - Given TINKERPOP-2132 seems to be solved by this body of work, I think this PR should include the tests I had in my branch (unless they duplicate tests already in this PR): https://github.com/apache/tinkerpop/commit/329979e4c609b42866f1af80eb02860f195093c5#diff-2eb4fd75aa20b9b615aa8c20f5b54aa7 As for the rest of the changes in that commit, I'm not sure if we need to change the protocol anymore so perhaps we can just close TINKERPOP-2132 when this issue merges? > Use one connection per request for Java client > -- > > Key: TINKERPOP-2205 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2205 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.3.6 >Reporter: Divij Vaidya >Priority: Major > > This issue is a tracking item for the conversation in the mailing list > [[1]|https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E] > which highlights multiple problems and shortcomings in the existing Java > client and proposes a design change in the client connection pooling to > address the same. More specifically, the problems addressed are as follows: > # Difficulty in configuring the client for optimum performance. > # Undocumented dependency of configuration parameters on each other. > # A bad request can impact other requests on the same channel. > # Host is marked as dead even if it is busy serving requests. > # No way to free up server resources if the client has stopped consuming > results. > # No differentiation between retriable and non-retriable exceptions from the > application code. > # Keep alive is only sent when a query is executing, which means that a > connection open for a very long time with no query being sent will be closed > by the server. > # Race condition if the server response reaches before result queue has been > registered. > # Unpredictable behaviour if the server sends an exception followed by a > genuine response for the same request. > # A concurrent hash map (tracking pending requests) is a point of contention > amongst threads. > [1]https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TINKERPOP-2205) Use one connection per request for Java client
[ https://issues.apache.org/jira/browse/TINKERPOP-2205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16830910#comment-16830910 ] ASF GitHub Bot commented on TINKERPOP-2205: --- divijvaidya commented on pull request #1105: TINKERPOP-2205 Change connection management to single request per channel URL: https://github.com/apache/tinkerpop/pull/1105 https://issues.apache.org/jira/browse/TINKERPOP-2205 The code in this pull request changes the server interaction mechanism of the Gremlin open source Java client. The new code addresses problems and shortcomings discussed in the linked conversation [[1]](https://lists.apache.org/thread.html/77728cb77d4eab90f15680595e653ffc6055b74db29cbd4dcd5f0339@%3Cdev.tinkerpop.apache.org%3E). More specifically, the problems addressed are as follows: 1. Difficulty in configuring the client for optimum performance. 2. Undocumented dependency of configuration parameters on each other. 3. A bad request can impact other requests on the same channel. 4. Host is marked as dead even if it is busy serving requests. 5. No way to free up server resources if the client has stopped consuming results. 6. No differentiation between retriable and non-retriable exceptions from the application code. 7. Keep alive is only sent when a query is executing, which means that a connection open for a very long time with no query being sent will be closed by the server. 8. Race condition if the server response reaches before result queue has been registered. 9. Unpredictable behaviour if the server sends an exception followed by a genuine response for the same request. 10. A concurrent hash map (tracking pending requests) is a point of contention amongst threads. ### Changes 1. ResultSet can be closed. * This allows the client to tell the server to relinquish resources associated with this request. 2. Single request per connection. No channel multiplexing. * Impact of a rogue response (such as one which causes IOException exceeding content length) does not impact the rest of the in-flight requests. * Each request has its own bandwidth. 3. Removed custom keep alive logic and replaced with Netty IdleState handler. * Makes the client more robust 4. Deprecated InProcess and SimultaneousUsage configuration parameters. * Now the customers would have to configure only a single parameter for setting concurrency of requests. 5. Throw different exceptions to the application code which makes it easy to determine what can be retried and what not. 6. Handle errors gracefully during WebSocket handshake. * Makes the client robust 7. Close the websocket channel gracefully (with a close frame). * Server closes the channel gracefully on receiving the close frame. 8. Use EPoll instead of Nio whenever possible. * Poll provides better performance on Linux platforms 9. Run chooseConnection in an async manner using executors threads. * Increases thread utilization. In general a lot of effort has been made to improve thread utilization. 10. Make client resilient to multiple response from the server for the same request. 11. Client operations do not rely on the UUID of the request provided by the server. ### Backward compatibility with 3.4.x/3.3.x **Application layer code** - This new client is fully backward compatible and requires no change in the application layer code. The only change required will be if the application layer code is relying on certain types of exceptions thrown by the client. **Channelizer** - Although the channelizer interface hasn’t changed, custom implementations of the channelizer will have to change their code to work with the new client. ### Limitations 1. A client generating high TPS from a single machine will have to modify the OS setting for max number of open files, since each connection corresponds to a single file in linux OS. ### Benchmarks Benchmark code will be shared soon in this PR and results will be updated here. During preliminary testing, there was no difference in performance. This is because channels are being re-used and the additional overhead is only at the bootstrap when we do more WebSocket handshakes (due to more connections) than older code. ### Testing 1. Added a new test suite. 2. All existing tests pass. * gremlin-driver: mvn clean install -DskipIntegrationTests=false * gremlin-server: mvn clean install -DskipIntegrationTests=false ### Post merge work 1. Write a document describing how the client works. 2. Add examples of efficient usage of client. 3. Update change log. 4. Update documentation. ### Future work 1. Add a default retry strategy for timeouts while trying to obtain a connection. 2. Add a strategy to remove a fishy host from the load
