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-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
