You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Roytman, Alex" <Ro...@peacetech.com> on 2002/11/01 19:04:07 UTC

DBCP: Jdbc2PoolDataSource needs attention

Dear apache-commons developers,

Jdbc2PoolDataSource is a very useful component however I believe it is not up to "release" quality yet
While working with it I found some things I would like to improve or change. 
Please forgive me if I misunderstood certain things in Jdbc2PoolDataSource design as I could only spend limited time testing, debugging and reading its source

1. mutable poolable keys are very dangerous and lead to errors. One subtle error which rendered entire component useless in case of getConnection(username, password) was due to this error http://issues.apache.org/bugzilla/show_bug.cgi?id=13235

2. new pool gets created for every different user getConnection(username, password). Also due to pool design if user wants eviction each pool will span an eviction thread which can easily bring any server to its knees. I think creating new pool for each user is related to ability to configure each user separately. My believe it is really not an objective because a) as much as possible in their projects people should use pools which are fully configured and do not use getConnection(username, password) but getConnection() instead and b) if they do need pool which caters for application where username/password is supplied by user for each session rather then configured in application this pool should have no reason to configure different users differently (as it has no prior knowledge of its users)
Also for this kind of pool it is important to be efficient as number of users can be very high
and we should be extra careful to survive programming mistakes or malicious users in this case

3. In current implementation once a pool for particular user/password got created and have some idle connection a user with INVALID password but right user name can grab connections from the pool (have not tested but it looks like it is the case)

4. failed connections get suck in active pool

5. Not sure why we need to keep a static map of all pools by datasource. J2EE environment will call jndi environment getObjectInstance() only once to create a pool and then the environment will keep the reference and will not call factory method on each object lookup but return previously created instance

6. weak cache of UserPassKeys by PooledConnections. if PooledConnections re-implements 

7. XA interface?

As an exercise (and because I need it ASAP for my projects) I re-implemented Jdbc2PoolDataSource (well I just wrote it from scratch yesterday). It is also based on commons-pool component and I tried to take in the consideration some of the issues I mentioned above. It is an RND/alpha level code and not all features are implemented but I am planning to finalize it shortly (actually it depends on pool latest features invalidateObject() and doEvict()) as I need it for my production. I expect to get it to beta quality in couple of weeks. I would be glad to contribute it to apache if there is any interest. 

some features:

- getConnection(username, wrong-password) will not affect pools for the same user name and valid password and will not be able to utilize them either 
- UserKey objects are immutable and cached in a map (HashMap right now but it could be apache's LRUMap to make sure it does not grow indefinitely)
- Bad connections do not get stuck in active pool
- Connections are marked bad based on ConnectionEventListener.connectionErrorOccurred() and get removed from a pool on next activate/passivate
- On getConnection() if any idle connection is bad it will be destroyed and another attempt to get connection made transparently for user. It will be done till a good connection is obtained but not more than idleNum+1 attempts to be made (tested with oracle)
- Use shared eviction thread across multiple pools. This has not been implemented yet. I am planning to allow pool to have setTimer(Timer timer) or setTimerName(String jndiName) which will subscribe pool to receive time events from  a timer. In j2ee environment timer should be defined in context environment and looked up by jndi name. Timer could be custom developed or preferable java.util.Timer (although as Rodney mentioned it requires jdk1.3)
- Optionally prohibit getConnection(username, password) allow getConnection() only (could be important for XA vs. local transactions) not implemented yet

and more...


Thank you very much for your attention

Alex Roytman
Peace Technology, Inc.
roytmana@peacetech.com

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: DBCP: Jdbc2PoolDataSource needs attention

Posted by "Craig R. McClanahan" <cr...@apache.org>.

On 10 Nov 2002, John McNally wrote:

> >
> > 5. Not sure why we need to keep a static map of all pools by datasource. J2EE environment will call jndi environment getObjectInstance() only once to create a pool and then the environment will keep the reference and will not call factory method on each object lookup but return previously created instance
> >
>
> Tomcat may do this, I do not know.  But I do not think a jndi service is
> required to return the same instance to every lookup.  Is there a
> specification that says J2EE containers will cache instances?  The jdbc
> spec mentions use of static fields as a way to code around the fact that
> there might be multiple instances which should be referring to the same
> pool of connections.
>

The situation with regard to caching instances (and related concerns about
the immutability of the instances being returned) was ambiguous in the
J2EE 1.2 and 1.3 specs, but has been clarified in J2EE 1.4 (Proposed Final
Draft), section 5.2:

    In general, lookups of objects in the JNDI "java:" namespace
    are required to return a new instance of the requested object
    every time.

There are some listed exceptions to this rule (primarily for things that
are known to be immutable (such as a java.lang.String) or designed to be
singletons (at the JVM level), but these exceptions don't apply to things
like JDBC data sources.  Looks like we'll have to change things in Tomcat
5.

Section 5.4 talks in more detail about the container's responsibilities
related to resource manager connection factories in a J2EE app server
environment.

You can grab the J2EE 1.4 PFD platform spec at:

  http://java.sun.com/j2ee/download.html


> john mcnally
>

Craig


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: DBCP: Jdbc2PoolDataSource needs attention

Posted by John McNally <jm...@collab.net>.
On Fri, 2002-11-01 at 10:04, Roytman, Alex wrote:
> Dear apache-commons developers,
> 
> Jdbc2PoolDataSource is a very useful component however I believe it is not up to "release" quality yet
> While working with it I found some things I would like to improve or change. 
> Please forgive me if I misunderstood certain things in Jdbc2PoolDataSource design as I could only spend limited time testing, debugging and reading its source
> 
> 1. mutable poolable keys are very dangerous and lead to errors. One subtle error which rendered entire component useless in case of getConnection(username, password) was due to this error http://issues.apache.org/bugzilla/show_bug.cgi?id=13235
> 

I have made the keys immutable.

> 2. new pool gets created for every different user getConnection(username, password). Also due to pool design if user wants eviction each pool will span an eviction thread which can easily bring any server to its knees. I think creating new pool for each user is related to ability to configure each user separately. My believe it is really not an objective because a) as much as possible in their projects people should use pools which are fully configured and do not use getConnection(username, password) but getConnection() instead and b) if they do need pool which caters for application where username/password is supplied by user for each session rather then configured in application this pool should have no reason to configure different users differently (as it has no prior knowledge of its users)
> Also for this kind of pool it is important to be efficient as number of users can be very high
> and we should be extra careful to survive programming mistakes or malicious users in this case
> 

A new pool only gets created for each user that is configured with a
perUserMaxActive value and possibly other perUser values.  This might be
used to have a user that can write and a readOnly user, for example. 
You state that separate DataSources should be setup for each of these. 
I don't know enough cases to determine if that is the case, if others
agree, we can drop the per user configuration, but I would prefer to
keep unless given more definitive reasons it is bad. But it would not
effect the number of threads as the overall number of pools will be the
same in either case.

> 3. In current implementation once a pool for particular user/password got created and have some idle connection a user with INVALID password but right user name can grab connections from the pool (have not tested but it looks like it is the case)
> 

I have fixed this.

> 4. failed connections get suck in active pool

I was not able to reproduce this, but if the mutable, poolable keys were
the problem, it should be fixed.  As soon as I get my changes into cvs
maybe you could repeat your tests?

> 
> 5. Not sure why we need to keep a static map of all pools by datasource. J2EE environment will call jndi environment getObjectInstance() only once to create a pool and then the environment will keep the reference and will not call factory method on each object lookup but return previously created instance
> 

Tomcat may do this, I do not know.  But I do not think a jndi service is
required to return the same instance to every lookup.  Is there a
specification that says J2EE containers will cache instances?  The jdbc
spec mentions use of static fields as a way to code around the fact that
there might be multiple instances which should be referring to the same
pool of connections. 

john mcnally


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: DBCP: Jdbc2PoolDataSource needs attention

Posted by Jim Seach <jw...@yahoo.com>.
Alex,

Your implementation could also allow us to take advantage of Oracle's
"lightweight connections" where the application server obtains a
connection to the database using a user id/password assigned to the
application, then (using a database specific feature) "impersonates"
the current user on the connection.  That way, we can take advantage of
user-specific access permissions implemented in the database but still
take full advantage of the connection pool.

Thanks,

Jim Seach

disclaimer for list: I work for one of Alex's clients, and this feature
will be very useful for us.

--- "Roytman, Alex" <Ro...@peacetech.com> wrote:
> Dear apache-commons developers,
> 
> Jdbc2PoolDataSource is a very useful component however I believe it
> is not up to "release" quality yet
> While working with it I found some things I would like to improve or
> change. 
> Please forgive me if I misunderstood certain things in
> Jdbc2PoolDataSource design as I could only spend limited time
> testing, debugging and reading its source
> 
> 1. mutable poolable keys are very dangerous and lead to errors. One
> subtle error which rendered entire component useless in case of
> getConnection(username, password) was due to this error
> http://issues.apache.org/bugzilla/show_bug.cgi?id=13235
> 
> 2. new pool gets created for every different user
> getConnection(username, password). Also due to pool design if user
> wants eviction each pool will span an eviction thread which can
> easily bring any server to its knees. I think creating new pool for
> each user is related to ability to configure each user separately. My
> believe it is really not an objective because a) as much as possible
> in their projects people should use pools which are fully configured
> and do not use getConnection(username, password) but getConnection()
> instead and b) if they do need pool which caters for application
> where username/password is supplied by user for each session rather
> then configured in application this pool should have no reason to
> configure different users differently (as it has no prior knowledge
> of its users)
> Also for this kind of pool it is important to be efficient as number
> of users can be very high
> and we should be extra careful to survive programming mistakes or
> malicious users in this case
> 
> 3. In current implementation once a pool for particular user/password
> got created and have some idle connection a user with INVALID
> password but right user name can grab connections from the pool (have
> not tested but it looks like it is the case)
> 
> 4. failed connections get suck in active pool
> 
> 5. Not sure why we need to keep a static map of all pools by
> datasource. J2EE environment will call jndi environment
> getObjectInstance() only once to create a pool and then the
> environment will keep the reference and will not call factory method
> on each object lookup but return previously created instance
> 
> 6. weak cache of UserPassKeys by PooledConnections. if
> PooledConnections re-implements 
> 
> 7. XA interface?
> 
> As an exercise (and because I need it ASAP for my projects) I
> re-implemented Jdbc2PoolDataSource (well I just wrote it from scratch
> yesterday). It is also based on commons-pool component and I tried to
> take in the consideration some of the issues I mentioned above. It is
> an RND/alpha level code and not all features are implemented but I am
> planning to finalize it shortly (actually it depends on pool latest
> features invalidateObject() and doEvict()) as I need it for my
> production. I expect to get it to beta quality in couple of weeks. I
> would be glad to contribute it to apache if there is any interest. 
> 
> some features:
> 
> - getConnection(username, wrong-password) will not affect pools for
> the same user name and valid password and will not be able to utilize
> them either 
> - UserKey objects are immutable and cached in a map (HashMap right
> now but it could be apache's LRUMap to make sure it does not grow
> indefinitely)
> - Bad connections do not get stuck in active pool
> - Connections are marked bad based on
> ConnectionEventListener.connectionErrorOccurred() and get removed
> from a pool on next activate/passivate
> - On getConnection() if any idle connection is bad it will be
> destroyed and another attempt to get connection made transparently
> for user. It will be done till a good connection is obtained but not
> more than idleNum+1 attempts to be made (tested with oracle)
> - Use shared eviction thread across multiple pools. This has not been
> implemented yet. I am planning to allow pool to have setTimer(Timer
> timer) or setTimerName(String jndiName) which will subscribe pool to
> receive time events from  a timer. In j2ee environment timer should
> be defined in context environment and looked up by jndi name. Timer
> could be custom developed or preferable java.util.Timer (although as
> Rodney mentioned it requires jdk1.3)
> - Optionally prohibit getConnection(username, password) allow
> getConnection() only (could be important for XA vs. local
> transactions) not implemented yet
> 
> and more...
> 
> 
> Thank you very much for your attention
> 
> Alex Roytman
> Peace Technology, Inc.
> roytmana@peacetech.com
> 
> --
> To unsubscribe, e-mail:  
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
> 


__________________________________________________
Do You Yahoo!?
Yahoo! Autos - Get free new car price quotes
http://autos.yahoo.com

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>