Issue Details (XML | Word | Printable)

Key: NUCCORE-575
Type: New Feature New Feature
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Andy Jefferson
Reporter: Fernando Padilla
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
DataNucleus Core

Option to retain datastore connection when operating non-transactional

Created: 30/Sep/10 11:36 PM   Updated: 23/Nov/10 11:00 PM   Resolved: 19/Oct/10 08:09 AM
Component/s: Persistence
Affects Version/s: None
Fix Version/s: 2.2.0.m2


 Description  « Hide
I bet we can discuss the pros/cons, but we've seen good performance gains from turning on a feature like this. I know that the connection pool bears the brunt of this, but even then, it's just wasting resources and increasing contention around the connection pool.


a) we switched to BoneCP from DBCP and saw a huge performance gain, since it was so much more efficient at
   get/release of connections
b) even so the connection pool and datanuclues both do different things when it gets/releases a connection from
   the pool, and this is just totally wasted cpu and roundtrips to the database



ps - we hacked our DN to enable a feature like this, and it does seem to help performance (as long as you have adequate connection pool/db connection settings). Sadly, we hacked it in such a way that we don't have a clean patch to submit. We can talk about what we did, but ultimately what you come up with will probably be better than what we did. :)


Sort Order: Ascending order - Click to sort in descending order
Fernando Padilla added a comment - 30/Sep/10 11:48 PM
From now on, I'm talking about the ConnectionManagerImpl.connectionPool

basically, we tried to follow what you currently do for "managed" ManagedConnection objects; since any ManagedConnection object that is "managed" will be added to the pool... But it is also used to associate a transaction context with the ManagedConnection. So we created a new variable called "pooled", so we could separate the two meanings of "managed"...

then we can add a connection to the pool, and have it last for the remainder of the PM's life ( or transaction close ), just as it does with "managed" connections right now.

We also added a check, so that when a connection comes out of the pool and there is an active transaction; that the connection has been properly associated with that transaction ( essentially moving it from the "pooled" state to the "managed" state ).

Andy Jefferson added a comment - 06/Oct/10 03:43 PM
This cuts across another JIRA for retaining the connection when committing a transaction so that the query results are still usable (and not needing to read all query results in there and then). See NUCRDBMS-190

Fernando Padilla added a comment - 06/Oct/10 04:33 PM
thank you. I'll follow NUCRDBMS-190 then.

But reading that over, it sounds like what he's asking for is more complicated than what we're asking for.. so not sure if I want to convolute the two bugs. But it really depends on how you're thinking to implement it.


I just want for a ManagedConnection to not release/close the underlying connection until the PM is closed for a performance reason. Pretty simple description.


I did see that the current code keeps the connection around when a transaction is open, and it has code to explicitly commit/close once a transaction is over. I would be fine keeping that as is. Recycling the connection on transaction revert/commit/close is fine with me to keep the change set and risk of regressions small.


We do not use multithreaded true, as I always thought that would be more trouble than it's worth, and would force huge concurrency management overhead when it's not necessary. His requirements seem very specific to trying to optimize Multithreaded true case...

Andy Jefferson added a comment - 08/Oct/10 11:54 AM - edited
So in ConnectionFactoryImpl, where it doesn't release the connection when in a transaction, add a check on a persistence property and don't release when not transactional too. So we then would have one connection for the transactional usage (tx ConnectionFactory), and one connection for non-tx usage (nontx ConnectionFactory).

This mode of operation (retain non-tx connection) has to be enabled by a persistence property since many people will not want connections to be retained (if they have many PM's then one per PM will soon exhaust any connection pool).

But then maybe there is more to it, and I'm waiting for you to attach your code so know what is "pooled" variable, and indeed what changes are made in ConnectionManager.

Andy Jefferson added a comment - 08/Oct/10 05:03 PM
Likely simpler is just to modify rdbms ConnectionFactoryImpl so that it makes use of "resourceType" to know when the factory is non-tx and that can check on a persistence property to decide whether to hang on to the connection (rather than close). Then modify "release" to do the commit on the connection, and not do a close() in that case. Maybe also update close() so that it only does commit+close when not retaining the connection. More complicated than that ?

Andy Jefferson added a comment - 11/Oct/10 03:23 PM
Give SVN trunk a try, and make use of the persistence property "datanucleus.connection.nontx.releaseAfterUse" which has a default of true.

**Not tested**

Andy Jefferson added a comment - 19/Oct/10 08:09 AM
Marking as fixed since in my simple test ... multiple queries, run non-tx ... it retains the connection and uses it in subsequent (nontx) queries

Tom Zurkan added a comment - 29/Oct/10 09:10 PM
testing the feature out with 2.2.0-m2.
can you add a check for null on the connection here:

java.lang.NullPointerException
at org.datanucleus.store.rdbms.ConnectionFactoryImpl$ManagedConnectionImpl.release(ConnectionFactoryImpl.java:281)
at org.datanucleus.store.rdbms.request.FetchRequest.execute(FetchRequest.java:370)
at org.datanucleus.store.rdbms.RDBMSPersistenceHandler.fetchObject(RDBMSPersistenceHandler.java:307)



Andy Jefferson added a comment - 30/Oct/10 10:37 AM
Done thx. obviously knowing what you do in your persistence code to get that would be nice ;-)

Tom Zurkan added a comment - 23/Nov/10 11:00 PM
just wanted to leave a note that i opened a bug on this feature.

http://www.datanucleus.org/servlet/jira/browse/NUCCORE-603

thank you!