[ https://issues.apache.org/jira/browse/POOL-86?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sebb resolved POOL-86. ---------------------- Resolution: Fixed > GenericKeyedObjectPool retaining too many idle objects > ------------------------------------------------------ > > Key: POOL-86 > URL: https://issues.apache.org/jira/browse/POOL-86 > Project: Commons Pool > Issue Type: Bug > Affects Versions: 1.3 > Reporter: Mike Martin > Fix For: 1.4 > > Attachments: pool-86.patch, pool-86.withtest.patch > > > There are two somewhat related problems in GenericKeyedObjectPool that cause > many more idle objects to be retained than should be, for much longer than > they > should be. > Firstly, borrowObject() is returning the LRU object rather than the MRU > object. > That minimizes rather than maximizes object reuse and tends to refresh all the > idle objects, preventing them from becoming evictable. > The idle LinkedList is being maintained with: > borrowObject: list.removeFirst() > returnObject: list.addLast() > These should either both be ...First() or both ...Last() so the list maintains > a newer-to-older, or vice-versa, ordering. The code in evict() works from the > end of the list which indicates newer-to-older might have been originally > intended. > Secondly, evict() itself has a couple of problems, both of which only show up > when many keys are in play: > 1. Once it processes a key it doesn't advance to the next key. > 2. _evictLastIndex is not working as documented ("Position in the _pool where > the _evictor last stopped"). Instead it's the position where the last > scan > started, and becomes the position at which it attempts to start scanning > *in the next pool*. That just causes objects eligible for eviction to > sometimes be skipped entirely. > Here's a patch fixing both problems: > GenericKeyedObjectPool.java > 990c990 > < pool.addLast(new ObjectTimestampPair(obj)); > --- > > pool.addFirst(new ObjectTimestampPair(obj)); > 1094,1102c1094,1095 > < } > < > < // if we don't have a keyed object pool iterator > < if (objIter == null) { > < final LinkedList list = (LinkedList)_poolMap.get(key); > < if (_evictLastIndex < 0 || _evictLastIndex > > list.size()) { > < _evictLastIndex = list.size(); > < } > < objIter = list.listIterator(_evictLastIndex); > --- > > LinkedList list = (LinkedList)_poolMap.get(key); > > objIter = list.listIterator(list.size()); > 1154,1155c1147 > < _evictLastIndex = -1; > < objIter = null; > --- > > key = null; > 1547,1551d1538 > < > < /** > < * Position in the _pool where the _evictor last stopped. > < */ > < private int _evictLastIndex = -1; > I have a local unit test for this but it depends on some other code I can't > donate. It works like this: > 1. Fill the pool with _maxTotal objects using many different keys. > 2. Select X as a small number, e.g. 2. > 3. Compute: > maxEvictionRunsNeeded = (maxTotal - X) / numTestsPerEvictionRun + 2; > maxEvictionTime = minEvictableIdleTimeMillis + maxEvictionRunsNeeded > * timeBetweenEvictionRunsMillis; > 4. Enter a loop: > a. Borrow X objects. > b. Exit if _totalIdle = 0 > c. Return the X objects. > Fail if loop doesn't exit within maxEvictionTime. > Mike -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira