tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cris Berneburg - US <cberneb...@caci.com>
Subject RE: Need help understanding DB connection versus servlet request life cycle
Date Tue, 18 Aug 2015 13:55:33 GMT
Chris,

Thanks for taking time to explain all that to me.  I appreciate it.  My responses inline below.

--
Cris Berneburg, Lead Software Engineer, CACI


-----Original Message-----
From: Christopher Schultz [mailto:chris@christopherschultz.net] 
Sent: Saturday, August 15, 2015 8:32 PM
To: Tomcat Users List
Subject: Re: Need help understanding DB connection versus servlet request life cycle

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Cris,
> 
> On 8/14/15 1:36 PM, Cris Berneburg - US wrote:
> > -----Original Message----- From: osagie uwaifo 
> > [mailto:osagieuwaifo@gmail.com] Sent: Friday, August 14, 2015 10:49 AM 
> > To: Tomcat Users List Subject: Re: Need help understanding DB 
> > connection versus servlet request life cycle
> > 
> >> Chris,
> >> 
> >> Why don't you allow tomcat manage the connection for you?
> >> 
> >> http://tomcat.apache.org/tomcat-7.0-doc/jdbc-pool.html
> >> 
> >> [SNIP]
> 
> You just need to code myBatis to use a JNDI DataSource. Then make sure that the DataSource
you configure in Tomcat (in your application's META-INF/context.xml file) agrees with the
DataSource's name you tell myBatis to use. Sometimes the exact syntax on either side is tricky
to get, since certain components like to add their own prefixes without making it abundantly
clear what prefixes are actually being added from the root of the JNDI context.

I just re(?)discovered that myBatis is already configured to use DBCP, which may mean I don't
need to set up a JNDI datasource after all:

<transactionManager type="JDBC"/>
	<dataSource type="POOLED">

I added these connection pooling properties below.  Hopefully that will help support DB connections
in the JSP layer, at least until they are later moved to the servlet layer like you suggest.

<property name="poolMaximumActiveConnections" value="30" />
<property name="poolMaximumCheckoutTime" value="60000" />
<property name="poolPingQuery" value="SELECT 1" />
<property name="poolPingEnabled" value="true" />

> It's not a good idea to keep a database connection open for the duration of the request.
For one, not all requests are going to require a database connection, and you'd be pulling
one out of the pool only to starve other requests that actually do need them. Second, the
request (usually) takes longer than the database transaction, so you are also starving other
threads by doing that.
> 
> I would recommend checking a connection out of the pool, performing a small transaction,
then returning the connection to the pool using Connection.close(). If you need to perform
multiple transactions, you can grab another connection if necessary.
> 
> If you find that you have multiple separate transactions per request that are stalling
waiting for connections, you might consider creating a utility method that handles that set
of transactions and does them all with a single connection.

Yeah, you're right, it's not very efficient.  However, we're a low-volume site, and we might
be able to get away with it until refactoring is finished.  The myBatis documentation (https://mybatis.github.io/mybatis-3/getting-started.html)
says this in the SqlSession section:

> consider the SqlSession to follow a similar scope to that of an HTTP request. In other
words, upon receiving an HTTP request, you can open a SqlSession, then upon returning the
response, you can close it.

Debugging, I see that my ServletRequestListener.requestInitialized is not fired for every
HTTP request, so it's not quite that bad.

On the other hand, the myBatis docs say this earlier in the same paragraph, which may explain
my "ExecutorException: Executor was closed" problems:

> Each thread should have its own instance of SqlSession. Instances of SqlSession are not
to be shared and are not thread safe. Therefore the best scope is request or method scope.
Never keep references to a SqlSession instance in a static field or even an instance field
of a class.

QUESTIONS:  I don't understand what parts have separate threads.  Does JSP rendering get its
own thread separate from the servlet?  Do Java tags embedded in JSP each have their own thread
separate from the JSP rendering?

Anyway, I changed the JSP pages to open their own SqlSession (instead of sharing from the
servlet) and explicitly close it when finished.  In the future I may also move the opening
of the SqlSession from the request listener to each servlet that needs one.

> Finally, you should take some time to remove all database accesses from your JSP code
and move it "back" into the servlet layer. That will give you a clean separation between your
controllers (the
> servlets) and your views (the JSPs). JSPs have fewer tools to allow you to handle errors,
etc. and so it's best to know that all is well before you try to generate a response -- because
once the response has been committed, you can't take it back and say "whoops, we actually
couldn't load that object from the database". Instead, you'll get either a broken page or
a silently-failed page where the user has no idea a problem occurred.

Again, you're right, and it's on my to-do list, but I don't have the time to complete that
task for this release, which needs to be delivered soon.  Basically I need to deploy this
as-is.  While it's not ideal, I'd like to think the software is in a better state than what
it was previously - as long as there are no catastrophic failures or performance issues.

> It sounds like your application has out-grown the initial sloppy design and could afford
some serious refactoring. It happens. I'm happy to hear that someone is actually taking a
look at the application and improving it.

It's taken me months to refactor the code so far to get it at its current state.  Previously
every SQL query returned a raw *recordset* - nothing previously was wrapped to return Java
objects, and no DB connections were *ever* explicitly closed!  Still have a ways to go.

> I would also recommend all of the following (because I wrote it, of
> course):
> http://blog.christopherschultz.net/index.php/2009/03/16/properly-handling-pooled-jdbc-connections/
> 
> All of that is still relevant if you are using myBatis, except that you won't be needing
to handle the JDBC calls yourself. At some point, you may decide that myBatis is not necessary
and you'll want to code your own SQL queries and write your own transactions. The techniques
in that blog post are equally relevant in either scenario.

Thanks, I read it again.  :-)  There's a lot of code that needs try-catch-finally wrapping.

> Hope that helps,
> - -chris
> 
> [SNIP]

Mime
View raw message