jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matej Knopp (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JCR-1456) Database connection pooling
Date Thu, 14 May 2009 19:30:45 GMT

    [ https://issues.apache.org/jira/browse/JCR-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12709547#action_12709547
] 

Matej Knopp commented on JCR-1456:
----------------------------------

>Using ThreadLocal for the connection seems unnecessarily complex (from the perspective
of someone new trying to understand the code), it's better to pass the connection around as
a method argument or encapsulate it as a member variable of an object that performs the database
operations 

The problem here is that BundleDbPersistenceManager superclass is connection agnostic so passing
the database connection as argument doesn't seem to be an option. Storing that as member variable
is, but it requires additional locking. To reduce unnecessary locking I decided to use thread
locals. Since it's perceived to be too complicated there won't be any in next patch.

> Could you implement this without introducing new configuration entries? We may consider
adding that later, but it would be clearer if we first implemented connection pooling with
the access configuration that we currently have. 

As far as I can remember all introduced configuration entities were completely optional with
sane default making the change pretty much transparent for user.

> We should leverage something like Commons DBCP instead of implementing our own connection
pooling logic. Commons DBCP is much better than anything that we could come up with. 

What's the point of hardwiring concrete connection pool? In my previous patch I didn't attempt
to do any connection pooling. I just had a connection source which could be easily implemented
to get the connection from *any* connection pool or JNDI. If you insist though on using commons
dbcp I can live with that though.

> Related to the above, we should use the standard DataSource interface interface instead
of a custom ConnectionManager class. This would nicely abstract away all the pooling logic
and make the code much more familiar to people who already know JDBC. All top-level methods
would look like something like this

The connection manager in patch didn't create database connections. It delegated that to a
connection provider. The purpose of ConnectionManager were some convenience methods and prepared
statements caching. The connection provider was simple DataSource like interface. The reason
why I chose not to use DataSource is because DataSource has no means to pass connection properties
(url, driver, etc). That normally isn't a problem, in jackrabbit however it is, because the
database properties are specified on components level (persistence manager, file system, journal,
etc). 

If PersistenceManager is supposed to have connection properties (which you seem to insist
on in order not to change configuration) then it needs a way to pass these information to
code that manages database connections. And DataSource doesn't provide any means for it.

The custom sandbox could work for me. 

Thanks for your input, it's very appreciated.

> Database connection pooling
> ---------------------------
>
>                 Key: JCR-1456
>                 URL: https://issues.apache.org/jira/browse/JCR-1456
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>          Components: jackrabbit-core
>            Reporter: Jukka Zitting
>         Attachments: patch-1456-1.txt, patch-1456-2.txt, patch-1456-3.txt
>
>
> Jackrabbit should use database connection pools instead of a single connection per persistence
manager, cluster journal, or database data store.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message