Win a copy of The Little Book of Impediments (e-book only) this week in the Agile and Other Processes forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Too many connections - Using AutoCloseables

 
Dustin Ward
Greenhorn
Posts: 24
1
Chrome Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
OK, so this might end up being wordy and slightly confusing, so I apologize. First, I am fairly new to Java, so I am jumping in to the deep end by working on a project. To try and be succinct, I am making a FoodTruckTracker program that uses JDBC to access mySql databse. I have tried to really abstract things out to make it less confusing, but now I have encountered a problem that makes my abstraction hard to fix. When I run my program, Eclipse gives me multiple errors, but the first is:
Data source rejected establishment of connection, message from server: "Too many connections"


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)...

I get this probably means I am not closing my queries correctly. The way I am writing my queries is to use the DAO design for each of my individual classes (FoodTruck, User, Menu and Ingredients each have a DAO and a corresponding table in my database). I was using prepared statements to sanitize my input and after I (sometimes) get a resultset, I would just close the prepared statement. The way I found to connect to the database was to use a class called Connect. I have two methods: connectToDB and close and in my Main Menu, I use these two methods to start and close the db connection in main (so... main(){ db.connectToDB ... //code ... db.close()} ).



In my DAO's, here is a sample: (FoodTruck)



I guess what I am asking is this: what is the best way to really connect / close my queries? I am not using multiple queries at once. It is a command line program where I get input, store it in the database and then reference that data later through more queries. But I don't think I need to implement a pool or anything like that. Thank you so much for reading all of this and I hope someone can help (sorry for any ignorance)!
 
Paul Clapham
Sheriff
Posts: 21567
33
Eclipse IDE Firefox Browser MySQL Database
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Bartender
Pie
Posts: 6503
83
  • Likes 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Greenhorn
Posts: 24
1
Chrome Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.


Ok, so I have lots of different classes and so I am trying to understand how to implement this. Is your suggestion to- in my MainMenu (where everything is the View)- get a connection like I do above (connection = database.getConnection()) and then how would I prepare the PreparedStatements? And where? I am using DAO for each class and they handle to call when data is needed to be accessed. Then it builds the prepstatements and fires it off. How would I build them and where would I store them? I guess that part is slightly unclear. I am all for writing the closing of whatever needs to be done if that makes it more efficient and I know a little bit about them, but my confusion is in implementation now. Hope that makes sense :)
 
Dustin Ward
Greenhorn
Posts: 24
1
Chrome Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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:


Hmm, that is interesting. So how does this work, exactly? Most of it looks understandable except what is going on with the dataSource part? And the repository just keeps all your SQL queries separate? Is there a benefit to this? What would you do, then with my Connect class that I have above? Can I scrap that or does that somehow tie in? Thanks so much for your help!
 
Stephan van Hulst
Bartender
Pie
Posts: 6503
83
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
A repository is just something that acts like a general container for a specific type of business object, without your business logic having to know about how the objects are stored exactly. In this case, the repository handles storing food trucks in a database through the DataSource interface, so that the rest of your code doesn't have to mess around with connections, statements, or SQL in general. A DataSource is just something that you can read data from or write data to, through connections. You configure it so that it uses the correct driver, credentials, etc., and then you just pass it around to any repository that needs to access the database.

The example repository opens a connection every time they need to read or write an object. Because the connection, statement and result set are all used inside a try-with-resources statement, they are automatically closed, so you don't have to worry about running out of connections.

Because opening connections is expensive, normally you wouldn't open and close one for every database access. Instead, you would keep them around for a longer period of time. DataSource.getConnection() doesn't actually give you a real connection, it gives you a smart object that when you try to close it, will just return the backing connection to the connection pool, which will close the real connection when it figures it's the right time for it.

So by using DataSource, you get connection pooling without having to do all the hard work, and you can just focus on closing your resources properly.
 
Roel De Nijs
Sheriff
Posts: 10662
144
AngularJS Chrome Eclipse IDE Hibernate Java jQuery MySQL Database Spring Tomcat Server
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Dustin Ward,

First of all, a warm welcome to CodeRanch!

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)...

In this topic you'll find a code example about how to incorporate the try-with-resources statement and benefit from the automatic resource management.

Hope it helps!
Kind regards,
Roel
 
Dustin Ward
Greenhorn
Posts: 24
1
Chrome Eclipse IDE Java
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hey everyone! Thank you for the help. Through your comments and trial/ error, I have finally not just figured out something that worked, but truly understand what was happening with my code and why it was throwing the errors.

First, I was using my Connect class in my Main class to open an initial connection to my database. Then, in my MainMenu class, I opened it, did stuff with my program (lots of stuff) and then it all came back to MainMenu and I closed the connection. Then, inside each DAO, I had almost the same code inside my constructors
On top of everything, I was just opening and closing prepared statements and was not even closing my connections inside my DAO classes (I had four or five). NO WONDER I was getting a 'Too many connections' error.

I resolved to trying a few things. First, I thought perhaps I could kind of get rid of my connections inside my DAO classes because my MainMenu opened and closed a connection, so why did I need more? Well, that doesn't really work. I found out rather quickly that because I was using prepared statements I needed the connection in each DAO and when I tried just declaring a connection, I was getting a NullPointerException because I hadn't initialized it. Ok, back to the drawing board. I adapted parts of code you all suggested and was able to get autocloseables with my prepared statements so I took out my connection in MainMenu and in the constructors of my DAO classes. I then added the connection code into the try-with-resources block and VOILA, it was not only working, but my code overall was much more elegant. I also found out that I was throwing Exception in my Connect class, so I had to put `throws Exception` eveywhere.... EW! When I fixed that with try-catches that were specific, it cleaned up ALL those throw Exceptions on my methods. SO MUCH NICER.

Here are the two code pieces I gave originally, but now working and without all the mess...




I still might be able to clean the code up faster with some of Stephan van Hulst's ideas (using a repository for all of my Prepared Statements) but thank you all for your help!!
 
Stephan van Hulst
Bartender
Pie
Posts: 6503
83
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
    Greenhorn
    Posts: 24
    1
    Chrome Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    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?


  • Yes, it has become 'noticeably' slower... its not slow, but it does hesitate when querying the database
  • 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?
  • Done and done!
  • Would e.printStackTrace() be better here? I just printed it out so I can see it and diagnose the problems


  • I am going to covert this to a GUI in a few days (when I finish)...
     
    Roel De Nijs
    Sheriff
    Posts: 10662
    144
    AngularJS Chrome Eclipse IDE Hibernate Java jQuery MySQL Database Spring Tomcat Server
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    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?

    Yes, you can simply remove that line. If the JDBC driver would not be on your class path, Class.forName("com.mysql.jdbc.Driver"); would have thrown a ClassNotFoundException at runtime

    Dustin Ward wrote:Would e.printStackTrace() be better here? I just printed it out so I can see it and diagnose the problems

    It is always better to use e.printStackTrace() than System.out.println(e);, because the latter will only print the class name and the error message, so that might result in losing very valuable information to troubleshoot any issue.

    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.
    So instead of havingyou could declare the SQLException instead of catching it. And then you could handle this exception in your service method or in the GUI controller and display the error message to the userBecause SQLException is a checked exception, it might result in cluttered code. And another alternative would be to create an unchecked exception and throw an instance of this unchecked exception wrapping the SQLException. And then you can decide when and how you handle this runtime exception without having to change your method signatures

    Hope it helps!
    Kind regards,
    Roel
     
    Dave Tolls
    Ranch Hand
    Posts: 2099
    15
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    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.
     
    Dustin Ward
    Greenhorn
    Posts: 24
    1
    Chrome Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    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.


    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?
     
    Dustin Ward
    Greenhorn
    Posts: 24
    1
    Chrome Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    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.


    That is very helpful. I always kind of wondered the difference between having a try - catch block and the throws extension
     
    Stephan van Hulst
    Bartender
    Pie
    Posts: 6503
    83
    • Likes 2
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    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?

    That's what the DataSource interface is for. If you get connections from DataSource instead of from your own connectToDB() method, you can then just pass a DataSource around that pools connections. Normally you use the connection pool of whatever web application container you're using, but for a stand-alone application you can include a library such as Apache Commons DBCP.

    DBCP includes a class BasicDataSource. Your application should create an instance of it, configure it, and then pass it to your DAOs/Repositories. It will pool connections for you, and you can just close connections using try-with-resources and everything will be fine.
     
    Dave Tolls
    Ranch Hand
    Posts: 2099
    15
    • Likes 3
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    A data source is the way to go (and for the desktop you can simply set it to 1 active connection) if you can.

    My description above (re: wrapping the connection) is to mimic aspects of the pool.
    The Connection you receive back is actually a wrapper around the real connection:


    If you just have the single Connection in your 'database' class then that can be empty.
    And you database class would look something like:



    Now, your other code can work as is, and (especially if the database class were to implement some suitable interface) it will be easy enough to swap this out for a DataSource, or whatever without having to change the calling code.

    I only put that lot in for completeness.
    If you can, try and set up a DataSource.
     
    • Post Reply
    • Bookmark Topic Watch Topic
    • New Topic