You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "upadhyay-prashant (via GitHub)" <gi...@apache.org> on 2024/05/11 02:55:34 UTC
[PR] Fixing connection cleanup in case of mass connection breaking. [tinkerpop]
upadhyay-prashant opened a new pull request, #2600:
URL: https://github.com/apache/tinkerpop/pull/2600
CAUSE:
In some cases when credentials expire, or servers encounters a
blip and closes all connections. The driver gets close message on all
connections. While processing those close messages, the driver was
getting into race conditions, where in multiple threads were trying to
close connections and trying to update the connections object i.e. list
of connections in the pool. This was leading to uncaught exceptions and
stale connections in the pool. These connections are never cleanedup
post this.
FIX:
Ignore the ArrayOutOfBoundException in trying to get an object from
connections object in case of creating a poolInfo message. This way,
the code tries to provide the accurate poolInfo best efforts basis.
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixing connection cleanup in case of mass connection breaking. [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2600:
URL: https://github.com/apache/tinkerpop/pull/2600#discussion_r1601811702
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java:
##########
@@ -665,13 +665,22 @@ private void appendConnections(final StringBuilder sb, final Connection connecti
final List<Connection> connections) {
final int connectionCount = connections.size();
for (int ix = 0; ix < connectionCount; ix++) {
+ try {
final Connection c = connections.get(ix);
- if (c.equals(connectionToCallout))
+ if (c.equals(connectionToCallout)) {
sb.append("==> ");
- else
+ }
+ else {
sb.append("> ");
-
+ }
sb.append(c.getConnectionInfo(false));
+ } catch (final ArrayIndexOutOfBoundsException ex) {
Review Comment:
i mentioned this to @upadhyay-prashant elsewhere but for others looking at this, i think we want to try to handle this differently as missing logs in the event of failure might prove to be confusing when debugging.
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixing connection cleanup in case of mass connection breaking. [tinkerpop]
Posted by "upadhyay-prashant (via GitHub)" <gi...@apache.org>.
upadhyay-prashant closed pull request #2600: Fixing connection cleanup in case of mass connection breaking.
URL: https://github.com/apache/tinkerpop/pull/2600
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixing connection cleanup in case of mass connection breaking. [tinkerpop]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2600:
URL: https://github.com/apache/tinkerpop/pull/2600#issuecomment-2105503241
## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2600?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 80.41%. Comparing base [(`9b46b67`)](https://app.codecov.io/gh/apache/tinkerpop/commit/9b46b6777d2fa250e41daacf2fa4554605aff53a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`fb95ecf`)](https://app.codecov.io/gh/apache/tinkerpop/pull/2600?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 86 commits behind head on 3.7-dev.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.7-dev #2600 +/- ##
=============================================
+ Coverage 76.14% 80.41% +4.26%
=============================================
Files 1084 27 -1057
Lines 65160 4978 -60182
Branches 7285 0 -7285
=============================================
- Hits 49616 4003 -45613
+ Misses 12839 773 -12066
+ Partials 2705 202 -2503
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/2600?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixing connection cleanup in case of mass connection breaking. [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2600:
URL: https://github.com/apache/tinkerpop/pull/2600#discussion_r1601811702
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java:
##########
@@ -665,13 +665,22 @@ private void appendConnections(final StringBuilder sb, final Connection connecti
final List<Connection> connections) {
final int connectionCount = connections.size();
for (int ix = 0; ix < connectionCount; ix++) {
+ try {
final Connection c = connections.get(ix);
- if (c.equals(connectionToCallout))
+ if (c.equals(connectionToCallout)) {
sb.append("==> ");
- else
+ }
+ else {
sb.append("> ");
-
+ }
sb.append(c.getConnectionInfo(false));
+ } catch (final ArrayIndexOutOfBoundsException ex) {
Review Comment:
i mentioned this to @upadhyay-prashant elsewhere but others looking at this, i think we want to try to handle this differently as missing logs in the event of failure might prove to be confusing when debugging.
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org