[
http://issues.apache.org/jira/browse/POOL-86?page=comments#action_12445951 ]
Mike Martin commented on POOL-86:
---------------------------------
Sandy wrote:
> I think the changes you suggest to the evict method will fail to make forward
> progress if getNumTests() is less than the number of idle objects for the
> current key. This is why the _evictLastIndex was needed.
_recentlyEvictedKeys can serve the same purpose. This change would do that:
--- GenericKeyedObjectPool.java.prev Tue Oct 31 10:24:44 2006
+++ GenericKeyedObjectPool.java Tue Oct 31 10:25:06 2006
@@ -1207,6 +1207,7 @@
return;
}
key = keyIter.next();
+ _recentlyEvictedKeys.add(key);
LinkedList list = (LinkedList)_poolMap.get(key);
objIter = list.listIterator(list.size());
}
@@ -1259,7 +1260,6 @@
}
} else {
// else done evicting keyed pool
- _recentlyEvictedKeys.add(key);
key = null;
}
}
> Back to the first part of your original submission: What I understand you
> really want is a way to prune the pool size down after a peak load spike.
>
> Would a decorator that either occasionally discards returned objects or uses
> some heuristics to determine when to discard returned objects meet your needs?
> This could be implemented so that it doesn't need an eviction thread which
> means we can guarantee thread-safety of the pool implementation but it would
> require some pool activity to do it's work.
I think what I want, and what most DB connection pools need, is the existing
idle eviction facility as advertised.
I've never encountered the thread-safety problems you mention. Is there an
outstanding bug regarding that? If so, it's not occurring in my environment
which as I said involves hundreds of keys (DB users), thousands of connections,
and in fact is used by hundreds of threads.
> GenericKeyedObjectPool retaining too many idle objects
> ------------------------------------------------------
>
> Key: POOL-86
> URL: http://issues.apache.org/jira/browse/POOL-86
> Project: Commons Pool
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Mike Martin
> Assigned To: Sandy McArthur
> 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 contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
