Skip to content

closeConnectionFully is performed without lock #115

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

Closed

Conversation

rPraml
Copy link
Collaborator

@rPraml rPraml commented Feb 28, 2025

Note: #127 would be our preferred solution


We now might have a better understanding what happened due our network outage.

In #114 we have already identified, that the connection.close may block.

In case of the DB2 driver (and maybe other drivers, too), the connection.close() will communicate with the database.
During closeConnectionFully we hold a lock, so that no other thread can obtain new connections (although if there are free ones)

In this PR we tried to do the closeConnectionFully outside of the scope of the lock. This should increase the multi thread performance of the pool.

Please take a look at these changes, if you would agree.

There are still two places, where the closeConnectionFully is called within the lock:

  • shutdown -> should not be a problem
  • reset -> may be a problem, because the pool will contiune with it's work only if the last connection is closed properly (if the connection pool is huge enough and you may estimate 30sec network timeout, this can take a lot of time)

So I'm unsure how to deal with the reset.
We assume, that restarting the pool took over 2 hours since our last crash.

We also added some more places, where logError is true (to be honest, I will always print the error in closeConnectionFully)

rPraml added a commit to FOCONIS/ebean-datasource that referenced this pull request Feb 28, 2025
Copy link
Collaborator Author

@rPraml rPraml left a comment

Choose a reason for hiding this comment

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

Hello @rbygrave

Have you had time to watch this and can you give me feedback?
I think we have found a problem why our application sometimes practically hangs.
As long as the pool is busy with CloseConnectionFully, no further connections will be issued (pool is completely blocked)

I'm unsure what to do in "reset"? This should not block or at least should not block too long.

@rPraml rPraml force-pushed the close-connection-outside-lock branch from bced7e8 to 255090d Compare April 14, 2025 12:13
@rPraml rPraml marked this pull request as draft April 16, 2025 09:57
@rPraml
Copy link
Collaborator Author

rPraml commented Apr 25, 2025

Closing, as #127 is merged

@rPraml rPraml closed this Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant