Data source rejected establishment of connection, message from server: "Too many connections"
Paul Clapham wrote:For a small application like that you shouldn't need to repeatedly open and close connections, and you should only need one connection. Not a connection pool, as you suspect.
And one thing about PreparedStatement is that it is reusable. Not only that, the database may "pre-compile" the SQL in a PreparedStatement so that subsequent uses after the first use run faster. So repeatedly creating and closing PreparedStatements may also be counterproductive.
So what you could do is this:
At the start of your application, before you do anything else, get a connection and prepare your PreparedStatements. Then while your application is running, use those PreparedStatements as required. But the ResultSet objects: they aren't reusable. So make sure to close them as soon as you're finished with them. Either use a finally block or (preferably, now) a try-with-resources statement. Then at the end of your application, you ought to close all of the PreparedStatements and close the connection. I say "ought" because it's not a desperate situation if you fail to do that; eventually the database will notice you haven't used them for a long time and will close them at its end. Not a problem in your environment, but in a professional environment your database manager will hunt you down and shout at you if you fail to close your connections promptly.
Stephan van Hulst wrote:Welcome to CodeRanch Dustin!
You're only closing your prepared statements. It's good practice to close the result sets and the connections separately. Since they're all AutoCloseable, you can use try-with-resources to do this. I find this works best if you use a connection pool and repositories. Here's an example:
Dustin Ward wrote:My question- before I make my wordy explanation- is how to re-implement some code so I can take advantage of AutoCloseables which I just found out about (and don't understand fully)...
Stephan van Hulst wrote:That looks a lot better. A few remarks:
Right now you're creating new connections on every database access. That might become prohibitively slow. Using Class.forName() to load a database driver is old-hat. I believe database drivers are automatically loaded these days, as long as they are on the class path. Assuming database is a member field of your DAO which you set with a field initializer, it's much better to set it from a constructor parameter. Don't catch exceptions prematurely. Does the DAO really know for sure that the client wants to print the exception message? What if we have a GUI and want to display the error message there?
Dustin Ward wrote:With Class.forName, do I just get rid of that? Um, forgive my ignorance, but how do I know if it is on the class path?
Dustin Ward wrote:Would e.printStackTrace() be better here? I just printed it out so I can see it and diagnose the problems
Dustin Ward wrote:
Yes, it has become 'noticeably' slower... its not slow, but it does hesitate when querying the database
Dave Tolls wrote:
Dustin Ward wrote:
Yes, it has become 'noticeably' slower... its not slow, but it does hesitate when querying the database
Is the GUI for this going to be a desktop one or a web app?
If it's desktop then you could just check if the connection is not null and return that.
You might, however, hit an issue with timeouts in your db over connections.
Note that you will probably want to wrap the Connection object so that you can do nothing with the call to close() as you won't want (in this case) to actually close the connection.
If it's a web app, then you should be using a connection pool.
Roel De Nijs wrote:But what Stephan tried to explain is that the Dao class might not be a good location to catch (and handle) your exceptions. Because if you already log the error in the insert() method (using e.printStackTrace() or System.out.println(e);), the exception is handled and you can't display the error message to the user in a GUI. But it all depends from your requirements.
Dustin Ward wrote:Just a desktop GUI...
I think I understand what you are suggesting... as you think it would be better to keep the connection open, but just have one continuous connection, right? When I was originally starting this project, that is what I was hoping to do, but I botched it with too many connections. How would I implement this, exactly? Would I check if the connection is not null in each DAO / each method in every DAO? And where would the open connection start? I also have a few methods that need information from other DAO's so it queries another DAO inside of the `while(rs.next())` loop. Is this acceptable and would that cause a problem?
There’s no place like 127.0.0.1. But I'll always remember this tiny ad:
a bit of art, as a gift, the permaculture playing cards
https://gardener-gift.com
|