On 9/11/14 11:39 AM, Phil Steitz wrote: > On 9/11/14 8:55 AM, Phil Steitz wrote: >> On 9/9/14 11:38 AM, Michael Berman wrote: >>> Great, thanks! I have all the new config property boilerplate in place and >>> I understand how it should work, but I'm debating the best way to implement >>> the per-key queues. It would be nice if the test iteration for each key >>> resumed in the same place the next time the cycle hits that key, but this >>> implies keeping around an iterator for every key. Right now, the test cycle >>> attempts to start from where it left off within the key, although it gives >>> up and moves onto the next key pretty aggressively if anything goes wrong, >>> and then starts back at the beginning the next time it comes back around. >>> Do you think that behavior should be preserved? >> I see the problem and I suspect current behavior won't work for what >> you or anyone else wanting this feature would like. Behavior would >> be OK for the key where the evictor left off, but visits would then >> revert to the same initial instances per key later on. I can't >> think of a reliable way around this other than maintaining iterators >> for each key, as you suggest. > Thinking about this some more, I am not sure the per key iterator > idea is so hot. This will be very tricky to maintain given that the > key list itself is dynamic. Other options would be to select the > instances to hit per key randomly somehow or just keep track of > "offsets" per key. The latter would not be precise or accurate > given pool membership changes, but would keep the evictor from > getting stuck hitting the beginning of the dequeue all of the time. > Random "offsets" might be a decent strategy for most practical use > cases and would be easy to implement.
I guess above is not appealing as it does not guarantee sequential order (or ever) evictor visits. I will summarize above to the ticket and push this issue out to 2.4 if thats OK. Phil > > Phil >> Phil >>> As far as maintaining iterators for each key goes, we could convert >>> poolKeyList, which is only read by the evictor process now into a >>> Map<K,Optional<Iterator<T>>> or something to track our place iterating >>> through the Deque for keys we've already seen (and just insert None when a >>> new key comes into existence, when we finish an iteration, or if we bail on >>> an iteration after coming across a borrowed object). Does that sound >>> reasonable to you? >>> >>> On Sat, Sep 6, 2014 at 5:22 PM, Phil Steitz <phil.ste...@gmail.com> wrote: >>> >>>> On 9/5/14 8:44 AM, Michael Berman wrote: >>>>> I was proposing having both properties be maxes, not mins; whichever we >>>> hit >>>>> first is the limit. >>>> Sorry, I misunderstood. That sounds fine to me. I would be happy >>>> to work with you on a patch implementing this. If you want to take >>>> a stab at it, just attach the patch (including unit test) to the >>>> ticket. >>>> >>>> Phil >>>>> I agree that not having a cap on the total number of >>>>> tests performed is dangerous (I wouldn't really set it to max int; just >>>>> something pretty high). However, there is some precedent for unbounded >>>>> tests: currently, if you set the limit to a negative, the number of tests >>>>> scales with the number of values. This doesn't seem fundamentally more >>>>> dangerous than scaling with the number of keys. >>>>> >>>>> In some situations, though, it's true that it's important to be able to >>>> cap >>>>> the total number of tests. By the same logic, I may also not want an >>>>> unbounded number of tests to be able to occur for each key, which makes >>>> the >>>>> "there will always be numTests performed regardless of the perKey >>>> setting" >>>>> option sound scary. Maybe there should be another switch for whether we >>>>> want the min or the max? >>>>> >>>>> >>>>> On Fri, Sep 5, 2014 at 11:28 AM, Phil Steitz <phil.ste...@gmail.com> >>>> wrote: >>>>>> On 9/3/14 8:53 AM, Michael Berman wrote: >>>>>>> Thanks for the feedback Phil! >>>>>>> >>>>>>> >>>>>>>> One way to do this would be to have the current property be a cap on >>>>>>>> the total number of tests performed, but the new one basically >>>>>>>> control the number done per key. So if, e.g. numTestsPerEvictionRun >>>>>>>> is 12, numEvictionTestsPerKey (or whatever we decide to call it) is >>>>>>>> 3, the evictor does 3 tests for each key and then moves on to the >>>>>>>> next immediately, up to 12. I guess if there are fewer than >>>>>>>> numEvictionTestsPerKey, the evictor just moves on. If the evictor >>>>>>>> gets all the way through before numTestsPerEcvictionRun it could >>>>>>>> wrap around or just stop. Would something like that work for you? >>>>>>>> Anyone have any better ideas? >>>>>>> "No more than numEvictionTestsPerKey per key and no more than >>>>>>> numTestsPerEvictionRun total" would definitely address my use case (in >>>> my >>>>>>> case, I'll just set numTestsPerEvictionRun to Int.MAX or something), >>>> but >>>>>> I >>>>>>> do wonder if that's what everyone would want. I could imagine expecting >>>>>> the >>>>>>> behavior to be "At least numEvictionTestsPerKey per key and at least >>>>>>> numTestsPerEvictionRun total" (which would be the behavior if we >>>> wrapped >>>>>>> around instead of just stopping). I suppose it's easy to make it clear >>>>>>> which behavior we have in documentation, but that presumes the one I >>>> want >>>>>>> is likely to be universally more useful than the alternative. Think I >>>> can >>>>>>> make that assumption? >>>>>> We would have to rename numTestsPerEvictionRun if we were to change >>>>>> it to mean a min. I would personally not like that. I would rather >>>>>> leave it defined as is and just have the new property control when >>>>>> the evictor moves from key to key. Another option would be to have >>>>>> the new property override the old one - so if numEvictionTestsPerKey >>>>>> is set (i.e. not the default), then numTestsPerEvictionRun is >>>>>> ignored and the evictor is controlled completely by the second one. >>>>>> >>>>>> Personally, I would favor the first of my suggestions above - >>>>>> numTestsPerEvictionRun determines the total number of tests >>>>>> performed and numEvictionTestsPerKey determines how many tests are >>>>>> done in each key set before moving to the next key. Not having a >>>>>> cap on the total number of tests performed is dangerous, as there is >>>>>> no bound to the number of keys so just doing numEvictionTestsPerKey >>>>>> for every key each evictor run could result in very long-running >>>>>> evictors. >>>>>> >>>>>> It would be good to get some other opinions on this. Anyone have >>>>>> ideas / preferences? >>>>>> >>>>>> Phil >>>>>> >>>>>> >>>>>>> With respect to naming: >>>>>>> numEvictionTestsPerKey is definitely more concise and readable than >>>>>>> numTestsPerEvictionRunPerKey, but I wonder if it makes it less obvious >>>>>> that >>>>>>> it's tightly connected to numTestsPerEvictionRun. In code completion, >>>> the >>>>>>> setters wouldn't appear next to each other, which might hurt >>>>>>> discoverability. Maybe just referencing each other in docs is enough to >>>>>>> mitigate that. Any votes one way or the other? >>>>>>> >>>>>> --------------------------------------------------------------------- >>>>>> 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 >>>> >>>> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
