c3p0
c3p0 copied to clipboard
swaldman
Orphanize overdue checked-out JDBC Connections
Hi,
I believe many like me didn't fully understand that long paragraph with bold face warning for unreturnedConnectionTimeout until either reading the code or having a morning epiphany -- c3p0 is making sure your application fails, mysteriously, asynchronously...
A more friendly way to tell the developer "Fix your $%!@% applications ..." would be to make the pool completely forget about the overdue the JDBC Connection, removing all references to the Connection. The Connection will be on its own. If it keeps a reference to the pool, then it's up to the pool to accept/reject the orphan Connection when the latter eventually checks in. What do you think?
Thanks
--Michael
c3p0 isn't making sure your application fails. That is what you are doing if you leak Connections. If you set unreturnedConnectionTimeout, c3p0 will kill your leaked Connections, defined as Connections not returned for a long period of time.
If your application relies on Connections that are intentionally held for long periods of time, then it will kill those too, and you cannot use unreturnedConnectionTimeout. But your application should not intentionally hold Connections checked out for long periods of time. The whole point of a Connection pool is to prevent all the hackery of earlier days when people tried to hold Connections open to avoid the cost of establishing new DBMS Connections. An application with a Connection pool should check-out Connections immediately prior to a database transaction, and check the Connection back in, immediately, promptly, reliably.
If you take care about that reliably part, you will never need unreturnedConnectionTimeout.
c3p0 cannot do what you suggest. I mean it could, but it'd be terrible in the general case, even if you'd find it helpful in your specific case. Unreturned Connections represent a Connection leak. If c3p0 simply excluded those Connections and went to the database to replace them, then Connectons would continue to leak ad infinitum. Resources -- memory, file handles, whatever else -- would continue to be consumed on the application side and on the DBMS side until everything turns to slush or molasses and eventually something breaks. Applications that for whatever reason cannot fix their Connection leaks need c3p0 to actually clean up those resources, which means killing the Connections.
It sounds like your application intentionally holds one or more Connections out for long periods of time. I'd encourage you to revisit that architecture. If you don't want to change your application, then yes, by no means use unreturnedConnectionTimeout, it basically amounts to scheduling an asynchronous break of your application, as you say. Better yet, if for some reason you wish to hold a Connection open for very long periods, skip the Connection pool and just fetch that Connection with DriverManager. (It's conceivable you'd have one very long lived Connection for some reason, and then ordinary ephemeral ones, in which case you might use c3p0 for the ephemeral ones, but manage the long-lived one yourself.) But holding a Connection open for very long periods of time is going to be a fragile architecture however you do it. Connections break. They time out, a network interruption occurs leaving them broken, etc. A Connection pool manages all that stuff for you. It tests Connections, and reacquires them when they break. Almost always, holding open a Connection is a bad idea.
It could be that your issue is that you occasionally hit very slow queries, despite checking out and checking in Connection promptly. In this case, the right thing to do is make unreturnedConnectionTimeout sufficiently large that it will almost never interfere with a potentially valid transaction.
Of course, the best thing of all to do is to temporarily use unreturnedConnectionTimeout in combination with debugUnreturnedConnectionStackTraces, find your Connection leaks, fix them, and then don't set unreturnedConnectionTimeout at all.
Hi Steve,
I understand and agree with the reasons of this feature and what ought to be done.
The symptom of the pool killing the overdue Connection is that the application throws an exception when it continues to use the dead Connection. Our first instinct is that there is a bug in our code or 3rd party code, not c3p0. So we spend a lot of time hunting for a bug that closed the Connection prematurely, never suspected c3p0.
People, including me, have predisposition about how a resource pool should behave, which is when the resource is checked out, the pool wouldn't interfere with the application. This c3p0 feature defies that general idea, for good reasons.
Perhaps an even stronger, more explicit and alarming language in log and documentation will help. When the pool kills the overdue Connection, it can log in ERROR or FATAL level acknowledging that the pool is forcefully killing the Connection still in used and the application may crash immediately.
Thank you
Michael Chen rivetlogic http://www.rivetlogic.com Voice +1.909.896.7763 Skype michael.chen_rivetlogic GTalk [email protected] Calendar michael chen's calendar http://www.google.com/calendar/hosted/rivetlogic.com/embed?src=michael.chen%40rivetlogic.com&ctz=America/New_York .
On Fri, May 5, 2017 at 11:19 AM, Steve Waldman [email protected] wrote:
c3p0 isn't making sure your application fails. That is what you are doing if you leak Connections. If you set unreturnedConnectionTimeout, c3p0 will kill your leaked Connections, defined as Connections not returned for a long period of time.
If your application relies on Connections that are intentionally held for long periods of time, then it will kill those too, and you cannot use unreturnedConnectionTimeout. But your application should not intentionally hold Connections checked out for long periods of time. The whole point of a Connection pool is to prevent all the hackery of earlier days when people tried to hold Connections open to avoid the cost of establishing new DBMS Connections. An application with a Connection pool should check-out Connections immediately prior to a database transaction, and check the Connection back in, immediately, promptly, reliably.
If you take care about that reliably part, you will never need unreturnedConnectionTimeout.
c3p0 cannot do what you suggest. I mean it could, but it'd be terrible in the general case, even if you'd find it helpful in your specific case. Unreturned Connections represent a Connection leak. If c3p0 simply excluded those Connections and went to the database to replace them, then Connectons would continue to leak ad infinitum. Resources -- memory, file handles, whatever else -- would continue to be consumed on the application side and on the DBMS side until everything turns to slush or molasses and eventually something breaks. Applications that for whatever reason cannot fix their Connection leaks need c3p0 to actually clean up those resources, which means killing the Connections.
It sounds like your application intentionally holds one or more Connections out for long periods of time. I'd encourage you to revisit that architecture. If you don't want to change your application, then yes, by no means use unreturnedConnectionTimeout, it basically amounts to scheduling an asynchronous break of your application, as you say. Better yet, if for some reason you wish to hold a Connection open for very long periods, skip the Connection pool and just fetch that Connection with DriverManager. (It's conceivable you'd have one very long lived Connection for some reason, and then ordinary ephemeral ones, in which case you might use c3p0 for the ephemeral ones, but manage the long-lived one yourself.) But holding a Connection open for very long periods of time is going to be a fragile architecture however you do it. Connections break. They time out, a network interruption occurs leaving them broken, etc. A Connection pool manages all that stuff for you. It tests Connections, and reacquires them when they break. Almost always, holding open a Connection is a bad idea.
It could be that your issue is that you occasionally hit very slow queries, despite checking out and checking in Connection promptly. In this case, the right thing to do is make unreturnedConnectionTimeout sufficiently large that it will almost never interfere with a potentially valid transaction.
Of course, the best thing of all to do is to temporarily use unreturnedConnectionTimeout in combination with debugUnreturnedConnectionStackTraces, find your Connection leaks, fix them, and then don't set unreturnedConnectionTimeout at all.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/swaldman/c3p0/issues/92#issuecomment-299538497, or mute the thread https://github.com/notifications/unsubscribe-auth/AEIE73NPtNCp2cp_lSmdeLWqouGqe8prks5r22gXgaJpZM4NSJdl .
--
CONFIDENTIALITY NOTICE: This e-mail, including attachments, is for the sole use of the intended recipient(s) and may contain confidential and privileged information or otherwise be protected by law. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender and destroy all copies and the original message.
Michael,
I'm very sorry for your debug hell. That sucks, and I feel bad. In general, c3p0 tries very hard to honor the norm that checkout out Connections are the applications, not to be interfered with by c3p0. Obviously and necessarily, unreturnedConnectionTimeout is an exception, a violation of that norm.
I'd love to put uglier logging and warning about unreturnedConnectionTimeout. I'd love to just kill the feature, which is bad, and was implemented very reluctantly after a lot of requests.
Unfortunately, it is my sense that the world is full full full of hibernate applications maintained by people several generations and contractors removed from their original authors. Many of these people are desperately afraid to touch the application code they don't understand. Rather than fix Connection leaks, they turn on unreturnedConnectionTimeout and have c3p0 clean up after them. Depending on the frequency of leaks, the size of the pool, the length of the timeout, and the intensity of use of the application, this may yield lumpy, spotty, performance as the DataSource temporarily freezes before leaked Connections are destroyed. But, often it is invisible, or at least works well enough, and frightened maintainers of old apps are happy with the solution. They would not be happy if I filled log at FATAL or SEVERE or ERROR with finger-wagging messages, as much as I want to. I could add sterner messaging while the combination of unreturnedConnectionTimeout and debugUnreturnedConnectionStackTraces is set, as the latter is a performance drag and should not be used in production, or at least should only be used temporarily.
You seem sane and smart. Can you eliminate whatever leak provoked the imposition of unreturnedConnectionTimeout and remove the setting? Then c3p0 will never be the culprit of mysterious application exceptions.
Perhaps this analogy can help developers understand the feature:
If you think of c3p0 as a pigeon farmer. You ask the farmer to give you a pigeon to send some messages and promise him to return the pigeon in one day. However, after one day, you are still using the pigeon to send messages. The farmer picks up a gun and shoot the pigeon dead in mid-air. That is what unreturnedConnectionTimeout=86400 means.
just feel i should mention, "no pigeons were killed in the making of this library."
to go with your analogy, if you set unreturnedConnectionTimeout, the value should be long enough that you are sure, if the pigeon has not returned after so long, it has defected and is now revealing secrets to the enemy. because after unreturnedConnectionTimeout seconds, c3p0 will indeed kill the pigeon, er, um, i mean Connection, presuming it is leaked and lost.