On 7/22/12 12:16 PM, Mark Thomas wrote: > On 21/07/2012 17:51, Phil Steitz wrote: >> On 7/20/12 1:47 PM, Mark Thomas wrote: >>> On 16/07/2012 20:55, Phil Steitz wrote: >>>> On 7/15/12 10:46 AM, Phil Steitz wrote: >>>>> On 7/15/12 1:12 AM, Mark Thomas wrote: >>>>>> On 15/07/2012 01:38, Phil Steitz wrote: >>>>>>> Sorry if this has been discussed already and I can't find the >>>>>>> thread; but we should do this now if we want to do it - i.e., move >>>>>>> DBCP's AbandonedObjectPool into [pool]. >>>>>> This is certainly something that - on the face of it - makes sense. >>>>>> However, when I looked at it a little while back it did not appear to be >>>>>> trivial to unpick the AbandonedObjectPool code from DBCP. >>>>>> >>>>>> I didn't spend enough time on it to come to a final conclusion. >>>>>> >>>>>> I can look at this in the next week or so. It would be helpful if other >>>>>> folks looked as well. >>>>> I just dug into this a little and it looks like just moving the >>>>> classes and adjusting imports will work. I had to increase >>>>> visibility of addTrace, removeTrace, get/setLastUsed in >>>>> AbandonedTrace; but other than that I did not have to make any >>>>> changes. If others are OK with this, I will open a JIRA for this >>>>> (maybe one for each of DBCP, POOL?), continue testing, cleanup the >>>>> docs and get the changes committed assuming I don't run into any >>>>> problems. Fortunately, the original design looks like it intended / >>>>> anticipated this move. >>>> OK, I have pretty much verified that this can work, but I am now >>>> asking myself if with the new GOP that holds references to checked >>>> out objects, do we need such a heavyweight approach? If we just >>>> extend PoolableObject to something like TrackablePoolableObject >>>> adding set/getLastUsed, the new GOP (extended) could do this >>>> directly (without adding the trace list). The only loss would be >>>> stack trace generation, which we could continue to provide in an >>>> abstract class implementing the new interface. The AbandonedTrace >>>> machinery in DBCP conflates trackability with maintaining >>>> parent/child relationships, which it could continue to do - i.e., >>>> leave AbandonedTrace in DBCP, implementing TrackablePoolableObject >>>> and perhaps inheriting from the abstract parent that supports stack >>>> trace generation. >>> Sounds good. I was thinking that recording the stack trace could be an >>> option of GOP/GKOP. i.e. add [g|s]etLastUsed to PooledObject and add an >>> option to BaseObjectPoolConfig to record stack trace on borrow. >> That will work for stack trace tracking. For lastUsed, the problem >> is that setLastUsed has to be available to the pool client (who >> needs to be able to call it while an instance is checked out). This >> is why AbandonedTrace exists (well, half of the reason). What I >> said above is a little garbled. I was thinking about >> PoolableConnections, but referring to "PoolableObjects" which don't >> actually exist in [pool]. So it seems we have two choices here >> (there may well be others, but two jump out at me): >> >> 0) Depart from the no-need-to-unwrap tradition in [pool] and hand >> out PooledObject<T> instead of <T> for abandoned object pools >> 1) Require that object factories used when abandoned object tracking >> is enabled produce objects implementing an interface including >> setLastUsed (or inherit from an abstract parent like AbandonedTrace) >> >> I don't like 0) for several reasons and 1) does not look that bad to >> me. Any better ideas? > I agree 0) is a poor choice. > > 1) Seems reasonable to me but the interface will need to include the > getter as well. > > That then leaves the question of where to put stack traces. a) in the > actual object or b) in PooledObject<T>? I'm leaning towards b) but we > could also provide a abstract base class. As I am unlikely to be doing > the work (up to my neck in Tomcat stuff at the moment) I'm happy with > whatever you choose.a
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? 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
