Re: [pool][dbcp] Move AbandonedObjectPool in v 2's?

[Phil Steitz <ph...@gmail.com>]

On 7/23/12 3:33 AM, Mark Thomas wrote:
> On 22/07/2012 22:41, Phil Steitz wrote:
>> One more quick question as I begin hacking:
>>
>> Currently, abandoned object recuperation is triggered by
>> borrowObject in AOP.   I am thinking about moving this to the
>> evictor.  Or I guess it could be in both places?
> The third option is a separate thread. That feels like overkill.
>
> The evictor is the natural place for this although not everyone wants to
> use an evictor. How feasible is use the evictor but do it on
> borrowObject() if an evictor is not configured?

I have created patches for both [dbcp] and [pool] that give us
something more concrete to talk about.  To start, I created
POOL-229, where I attached both patches.  Once we agree on the
approach for [pool] and commit the patch there, I will create a
[dbcp] issue and move the [dbcp] patch there.  Lets continue the
discussion here and I will summarize conclusion in the JIRAs.

I made the following changes:

Changes in [pool]
------------------
0) I kept the current AbandonedObjectPool cleanup behavior - if the
removeAbandoned property in the config is true, fire on borrow (with
the same pool depletion test); otherwise fire only in the evictor. 
The naming may be a little misleading, but this matches current
behavior and allows both choices.

1) Stack trace generation / persistence has been added to
PooledObject.  What triggers it is supplying a PrintWriter to the
constructor.  The pool provides this when it has an AbaondedConfig
defined including a logwriter.  Abandoned object cleanup is now just
an option of GOP.

2) A TrackedUse interface has been added including getlastUsed. 
Pooled objects implementing this interface will have their
abandonment determined via this method. Abandoned object pooling
will work with other objects, but classes that don't implement the
interface can't be queried for last used times, so abandonment is
just determined by how long an object has been checked out (i.e.,
abandonement == being checked out for longer than the abandoned
timeout).  A getLastUsed method has been added to PooledObject which
delegates to the implementation of this method in TrackedUse if the
objects being pooled implement that interface.  Slightly smelly, but
allows both kinds of use in GOP.

3) A getState method has been added to PooledObject.  Abandoned
object cleanup in GOP needs this because I did not add separate
tracking of checked out objects - i.e., allObjects is traversed,
using getState to identify the checked out objects for examination. 
We also need to check if the state has changed before actually
killing an abandoned object.

4) I kept TestAbandonedObjectPool for dbcp as a separate test class,
moved into [pool].  All tests pass.

Please review carefully the synchronization and checks in the code
to make sure objects deemed abandoned do not get destroyed after
being checked back in or other races.

Changes in [dbcp]
----------------
0) AbandonedTrace now just implements TrackedUse along with
setLastUsed methods and does not maintain stack traces, nor
AbandonedConfig.  The parent-child hierarchy and associated firing
of setLastUsed in subclasses is unchanged.

2) BasicDataSource creates a GOP with an AbandonedConfig, but
otherwise the AbandonedConfig instances passed around by factories
and pooled objects have been removed.

Abandoned connection tests and all others pass.

Suggestions for improvement welcome!

Phil


> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org