[pool][dbcp] Broken eviction cursors, LIFO/FIFO behavior (POOL-86)

[Phil Steitz <ph...@gmail.com>]

There are two problems with pool eviction that I want to fix for pool
1.3.1/dbcp 1.3 and I could use some feedback / opinions on options.

1.  The change in r332379 to remove the dependency on commons
collections broke the eviction cursors for GenericObjectPool and
GenericKeyedObjectPool.  Prior to this change (released in pool 1.3),
idle object evictors for these classes kept track of where they were
in the idle object list using "cursors" provided by the
o.a.c.collections.CursorableLinkedList, which was the backing store
for the idle object pools.  These cursors adjust for
additions/deletions to the list.  To eliminate the collections
dependency, the backing store was changed to an ordinary LinkedList
and to maintain evictor position state across runs an integer position
pointer member was added.  Unfortunately, this only works if the idle
object list does not change size between runs.

2.  Both pools were changed in 1.3 to be FIFO, whereas they had been
LIFO before.  This means that to hit oldest objects first, eviction
runs / cycles should start at the beginning, rather than the end of
the lists.

Both of these problems are mentioned in POOL-86.  There is also a case
made there that at least for DBCP, the prior behaviour (LIFO) is
better; but opinions appear to be mixed on this.

To fix the eviction cursor problem, we can

a) add (tricky) code to keep the integer pointer correct; or
b) revert to using o.a.c.collections.CursorableLinkedList.  We could
bring this class back in package scope without re-introducing the
[collections] dependency.
c) give up the attempt at marching through the entire list and always
start the evictor at the end (with oldest objects). This makes better
sense, actually, for *idle object eviction*, but the evictor does not
separate the concerns of evicting expired objects and validating idle
objects and the latter requires a (correctly positioned) persistent
cursor.

My vote is b).

I would also like to add a lifo property and allow LIFO/FIFO to be
configurable, with LIFO the default (changed back to 1.2 behaviour).
DBCP 1.3 would also expose the property, but default to LIFO.

Thanks in advance for feedback. Patches also welcome, of course.  If
there are no objections, I will go ahead with b) above and adding the
lifo property.

Phil

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org