tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Elli Albek <e...@sustainlane.com>
Subject Re: ConnectionPool question
Date Fri, 06 Nov 2009 22:03:10 GMT
Hi Josh, you are keeping this thread more and more interesting :)

This seems like a complex case. Passing open result sets directly to
pages makes things little more difficult, since the database code is
not handled in one place. Closing the connection via the result set
can lead to some problems, but it will indeed close the connection.
One possible problem is that the same connection was used for creating
two result sets. On the page you close the connection via the first
result set, this forces all open result sets to close, including the
second one which you still need to use.

In a typical Java application developers use doe design pattern, such
as MVC to separate the back end from the front end. The back end java
classes do the database queries (the model), and pass the data to the
page as java objects and not as live database objects. This makes the
pages simpler and without control flow complexity that open resources
cause (such as what you do when the database code throws errors and
the page is half written to the browser).

If all you have is the result set, you can either use it to close the
connection, or switch to the thread local system where you rely on the
thread local + filter combination to do all the resource management.

If you have many database queries to do, use the same connection. When
you get the strings and integers, close the result sets and
statements, but try to use the connection for as long as possible. In
our (home built) framework, this is done automatically. This is a
typical pattern, where server code has means to start and close a
connection implicitly without repeating the same code everywhere. Some
frameworks do it through annotations. How about something like this:

public Something getSomethingFromDatabase(Connection c) throws SQLException{
	PreparedStatement s = c.prepareStatement("sql");
	try{
		ResultSet rs = s.executeQuery();
		try{
			if (rs.next() == false)
				return null;
			int someNumber = rs.getInt(1);
			String someString = rs.getString(2);
			return new Something(someNumber, someString);
		}finally{
			rs.close();
		}
	}finally{
		s.close();
	}
	// connection left open
}

See this page for more ideas:
http://commons.apache.org/dbutils/examples.html

Simple rule: A method closes what it opened, and does not close what
it got from the outside. Keep this simple rule and your code will be
easy to follow and contain less errors.

But you also have to think what you try to achieve. Do you want to get
it to work, or go you want to make it solid. If it’s the first, then
most of this discussion is not relevant, the static getConnection and
filter close() solve it in a very simple and performing way without
touching the rest of the code. You can leak all over the place, it
will not make much difference. One connection, always closed. If it’s
the latter, then start looking at simple java database frameworks and
design patterns. That code looks like something that needs to be
rewritten rather than patched.

Getting the connection via the static method is fine. The addition of
thread local can also be easily done in the static method. What the
method does is get the data source and use it. If you want to use the
datasource directly, you will end up writing that code a few times.
The static method also gives you a central place to fix bugs and
control the system. It is the way the code is currently written, so
why change. It also gives you a place to add some extra logic, such as
getReadConnection() and getWriteConnection(), easy change.
The real solution will be to switch to something nicer. Example is a
framework that will give you an implicit connection via annotation on
a method, but this is more of a rewrite project.

It is not common to pass open result sets from the back end (model) to
the front end (view) and expect the view to close things correctly.
This gives you less ability to handle problems. For example exception
handling can be quite complex on the page. A page can get an error
from the DB layer when half of the page is written and flushed to the
browser. Now go figure out what to do.



On Fri, Nov 6, 2009 at 6:31 AM, Josh Gooding <josh.gooding@gmail.com> wrote:
> So with this being said, and I have to ask now the following questions:
>
> 1 - with statements that are returned directly (return
> ConnectionPool.getConnection().prepareStatement(sqlStatement);) how do I
> close the connection?  Since there are no finally blocks to close the
> connection in the method on the backend, would I do this in the presentation
> layer, and just do a rs.getStatement().getConnection().close();?  (or
> something like that?)
>
> 2 - Should each of my methods that call the DB and extract a string,
> integer, etc, have the "getConnection" in that particular method grab the
> info and then close the connection in the same method?
>
> 3 - Since I fixed / updated / and realized now it's a bad idea to write a
> wrapper for the CP, what do I do to get a connection?  Since I'll not be
> using the CP Wrapper anymore.  Would it be importing the datasource class
> and doing a ds.getConnection() right there in each method?
>
> 4- For my dumb curiosity what problems can it lead to?
>
> - Josh

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Mime
View raw message