History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: CASTOR-980
Type: Improvement Improvement
Status: Open Open
Priority: Minor Minor
Assignee: Unassigned
Reporter: Martin Fuchs
Votes: 0
Watchers: 0

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

optimize Castor transactions by avoiding to re-open JDBC sessions

Created: 02/Jan/05 08:56 AM   Updated: 25/Jan/06 06:24 AM
Component/s: JDO
Affects Version/s: 0.9.6
Fix Version/s: None

Time Tracking:
Not Specified

File Attachments: 1. Text File castor-1843.patch (6 kb)

Operating System: All
Platform: All

Bugzilla Id: 1843

 Description  « Hide
Currently Castor closes JDBC connections at the end of each transaction, and
creates another session for each following transaction. This is very time
consuming (if not using connection pooling) and might be avoidable when
optimizing the Castor behaviour regarding transactions and JDBC sessions.
Don't closing JDBC sessions at every transaction end can enhance the overall
performanc greatly.

For a illustration of my idea please see this link:

 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Martin Fuchs - 04/Jan/05 06:21 AM
Created an attachment (id=842)
first version of a patch

Could please someone with experience about using Castor in a EJB container look
into this patch and test, if also works in this environment?

Werner Guttmann - 06/Jan/05 12:44 PM
Martin, there's actually a lot of bugs out there that discuss refactoring all
this connection closing business, incl. limitations of the current
implementation, reasons why it's done one way or the other, etc. Can you please
read up on this ? Having said that, touching any of these methods is serious
business, according to Thomas Yip (the author of this code, because of
implementation details of various ejb/j2ee web containers).

Martin Fuchs - 07/Jan/05 05:05 AM
Yes, reading bugs 1491 and 1624 it seems to me, I tapped into a wasp's nest with
this bug.

Stein M. Hugubakken - 07/Jan/05 05:21 AM
I've been thinking of making the transactioncontext plugin-based, that way it's
better to add work-arounds for different databases.

Stein M. Hugubakken - 07/Jan/05 05:23 AM
BTW Martin,

If you type bug and number, bugzilla will create a hyperlink automagically

Like bug 1491 and same goes for comments, like comment 1

Martin Fuchs - 07/Jan/05 05:44 AM
You are thinking of one class replacing the current TransactionContextImpl with
_globalTx=true and another class replacing TransactionContextImpl with
_globalTx=false ?

Yes, this would make the code clearer.

> If you type bug and number, bugzilla will create a hyperlink automagically
Bugzilla should learn more correct grammatics and also understand "bugs".

Stein M. Hugubakken - 07/Jan/05 05:53 AM
> ------- Additional Comments From martin-fuchs at gmx dot net 2005-01-07 05:44 -------
> You are thinking of one class replacing the current TransactionContextImpl with
> _globalTx=true and another class replacing TransactionContextImpl with
> _globalTx=false ?

Something in that direction...

Werner Guttmann - 10/Jan/05 03:24 AM
> Something in that direction...
Plus more .. . I think this will be one of the first areas we should revisit
after the 0.9.6 release, as there's various public and local solutions floating
around, and any of those will solve at least two issues related to this
discussion. My personal opinion, as stated already elsewhere (read, I cannot
remember where .. ) is that the current TransactionContext/-Impl business
should be replaced with an interface/implementation business, making code
stricter, shorter and more concise.

Werner Guttmann - 13/Oct/05 09:05 AM
Martin, re-reading some very old issues, I wonder whether such a feature us really required. I mean, using a connection pool should be straight-forward these days with the availability of e.g. Jakarta Common DBCP, and in such an environment, the performance gain should be ignorable. Please, let me know what you think of this my comment.

Werner Guttmann - 24/Jan/06 03:50 PM
Martin ?

Martin Onis - 24/Jan/06 04:26 PM
I tried to find a way and explain it how Castor could avoid closing and re-opening database sessions. Using a connection pool improves the situation, but my target was to make things as simple as possible and avoid any unnecessary calls outside the library. One year ago you explained my approach is not sufficient, but the issue should be embedded a upcoming TransactionContextImpl rewrite. It seems this did not take place. Well, as I didn't find time and occasion to look further into it, my patch didn't get better. If you think it's not the direction in that Castor should go, then you might deactivate this Jira issue.

Werner Guttmann - 25/Jan/06 04:39 AM
Martin, I honestly didn't want to challenge the validity of your suggestion, if that's how my comment came across. If that's what happened, let me please apologize, as that has not been my intention. I agree with you that minimizing external dependencies is by definition a valuable approach. But at the same time, I wonder whether "re-inventing" the wheel in one way or the other is the right approach. And as you have mentioned yourself, we are all limited by time constraints .. . Unfortunately, days here in Europe last 24 hours only ....and I am not sure whether this can be changed any time soon.. Let's have an open discussion, and please rest assured that I have not come to any decision yet.

Ralf Joachim - 25/Jan/06 05:30 AM
For those using drivers this patch provides a big performance improvement but also for datasources with connection pooling a small improvement may be achived. To me this seams to be a quite simple patch. The only drawback is that the user needs to call db.close() to release connections but he should anyway do this to release other resources. This also sounds to me as a step towards the support of checkpoints which we should support anyway.

Werner Guttmann - 25/Jan/06 06:24 AM
Btw, just for completeness, the TransactionContext(Impl) classes have been refactored into an interface (TransactionContext), a base implementation and two separate implementation classes for local and global transactions, all to be found in the org.castor.persist package.