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:ch...@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:osagieuwa...@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]
