Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing connection cleanup in case of mass connection breaking. #2600

Open
wants to merge 1 commit into
base: 3.7-dev
Choose a base branch
from

Conversation

upadhyay-prashant
Copy link
Contributor

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.

 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.
@codecov-commenter
Copy link

codecov-commenter commented May 11, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 76.77%. Comparing base (9b46b67) to head (fb95ecf).
Report is 86 commits behind head on 3.7-dev.

Files Patch % Lines
...pache/tinkerpop/gremlin/driver/ConnectionPool.java 50.00% 2 Missing ⚠️
...rg/apache/tinkerpop/gremlin/driver/Connection.java 80.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             3.7-dev    #2600      +/-   ##
=============================================
+ Coverage      76.14%   76.77%   +0.62%     
- Complexity     13152    13178      +26     
=============================================
  Files           1084     1086       +2     
  Lines          65160    66293    +1133     
  Branches        7285     7303      +18     
=============================================
+ Hits           49616    50894    +1278     
+ Misses         12839    12678     -161     
- Partials        2705     2721      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sb.append(c.getConnectionInfo(false));
} catch (final ArrayIndexOutOfBoundsException ex) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants