You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@commons.apache.org by Tom <tb...@tbee.org> on 2006/11/20 13:25:38 UTC
[POOL] a conceptual implementation error?
Bear with me on this one...
The pool implementation splits pools into two parts: a provider of new
objects (PoolableObjectFactory) and a pooling behaviour (Pool). This is
a good idea, since this allows separation of content and behaviour: the
same source of poolable objects can be used in a round-robin or stacked
behaviour.
Now, in the current implementation, when the pool runs dry, the active
pooling behaviour can do one of three things: wait, create a new one, or
fail. For example: GenericObjectPool has settings to specify what
behaviour should be used: WHEN_EXHAUSTED_BLOCK, WHEN_EXHAUSTED_GROW,
WHEN_EXHAUSTED_FAIL. Behaviour 1 and 3 are straight forward, but "GROW"
is what I am having problems with.
If we consider GROW behaviour in the current implementation, we can look
at this as two sets inside the pool: one set with actively pooled
objects (those managed by the pool and on this pool the max active, etc
applies) and a second set with so far unpooled objects. The unpooled
objects may not even be instantiated, but theoretically they exists.
When the pool runs dry in GROW behaviour and thus needs a new object,
one object is moved from the unpooled set to the pooled set.
I consider this two-set setup an implementation detail: conceptually the
pool consist of both the actively pooled and unpooled objects, the user
of the pool just request a new object and the pool then must figure out
how to get one and whether or not to juggle between the sets.
Now to my point: in the current implementation this GROW behaviour ONLY
applies to the pooled objects set. An that is IMHO incorrect. What
should happen is: if the pooled objects set has run dry must ALWAYS try
to fetch from the unpooled set, only if the unpooled set also has run
dry, then the pool must decided wheter to BLOCK or WAIT. So it does not
matter if the pooled set ran dry, BOTH set must have run dry in order to
BLOCK or WAIT. Currently the StackedObjectPool goes into a very CPU
consuming busy-waiting loop, and the GenericObjectPool just acceps null
as the value.
As state above, the behaviour should be applied on the combined set, not
just the pooled set. Initially it seems that the behaviour should be
extended to the unpooled set, however if you consider this for a moment
longer, it turns out that the GROW behavour is actually not needed.
Given that the implementation detail of the pooled and unpooled sets is
of no concern to the user of the pool, he has no business in defining if
a pool should GROW. Either the pool is infinite (the factory can endless
continue creating new objects) or it is not. If the pool is infinite,
BLOCK and WAIT are of no concern. If the pool is not infinite and runs
dry, the pool can either WAIT for an object to be returned or FAIL.
There is no GROW.
So, to summarize:
- Pools handle the pooled object set according to specs (max active, min
active, etc).
- Pool always GROW, its is the factory that decides if there is a new
object available or not.
- If the factory says there is no more object (the pool ran dry), then
the pool decides if it will WAIT or FAIL.
- The pool may decide that certain pooled objects are returned to the
unpool set (destroy), when for example the "max active" is overflow.
Now, tell me if I see this wrong or not. As said before: currently the
StackedObjectPool goes CPU mongol on a dry pool and the
GenericObjectPool just returns null. I currently consider both
implementations seriously bugged.
Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [POOL] a conceptual implementation error?
Posted by Tom <tb...@tbee.org>.
Sandy McArthur wrote:
> On 11/21/06, Tom <tb...@tbee.org> wrote:
>> > It's non-trivial to accept code that wasn't written by Apache members
>> > because of licensing issues but I'm happy to look and learn from a
>> > good idea when I see one.
>>
>> Interesting. I didn't know that. I'll put it up once I add the priority
>> pool. An issue you mean a "bug" report in the commons area?
>
> Add an issue to http://issues.apache.org/jira/browse/POOL
>
Done.
BTW, if you like the implementation and if it makes things easier: I
have no problem with becoming an Apache member so I can commit directly.
Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [POOL] a conceptual implementation error?
Posted by Sandy McArthur <sa...@gmail.com>.
On 11/21/06, Tom <tb...@tbee.org> wrote:
> > It's non-trivial to accept code that wasn't written by Apache members
> > because of licensing issues but I'm happy to look and learn from a
> > good idea when I see one.
>
> Interesting. I didn't know that. I'll put it up once I add the priority
> pool. An issue you mean a "bug" report in the commons area?
Add an issue to http://issues.apache.org/jira/browse/POOL
--
Sandy McArthur
"He who dares not offend cannot be honest."
- Thomas Paine
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [POOL] a conceptual implementation error?
Posted by Tom <tb...@tbee.org>.
> Okay, I think I now have an idea of what you are trying to achieve. In
> terms of Pool you would want a pool with a maxActive setting set to 4
> and make sure there is no minIdle or idle object evictor or anything
> else that could cause the pool to loose poolable objects.
Correct. Only I think it is a bit unstable to have a factory producing
max 4 poolable objects and as an absolute requirement have the
associated pool handling set to maxActive = 4. If either one gets
changed the solution goes AWAL (meaning CPU 100%).
So in order to maximize user experience, the alternative pool
implementations have three constructors: one empty, one with a factory
and one with a collection, for example:
StackObjectPool lPool = new StackObjectPool( myList );
StackObjectPool lPool = new StackObjectPool( new
JdbcConnectionFactory(...) );
RoundRobinObjectPool lPool = new RoundRobinObjectPool( myList );
RoundRobinObjectPool lPool = new RoundRobinObjectPool( new
JdbcConnectionFactory(...) );
With this approach I have a clear separation between things-to-be-pooled
and how-the-pool-behaves, and can mix and match them.
> Pool assumes that poolable objects could become invalid over time. If
> makeObject cannot create objects as needed, the the pool would run
> dry.
That is a good assumption: all poolable objects should go through the
activate/validate/passivate life cycle.
>
> If you want to share those implementations, create an issue and attach
> the sources. I cannot promise it will be included in the official
> package but we can point others there if they have similar needs.
I want to add a PriorityObjectPool before that. Since I've been using
Apache's implementation for years I feel I should be contributing back.
And it's quite simple to do, below is the round robin implementation:
protected List iListOfPooledObjects = new ArrayList();
protected Object obtainPooledObject()
{
return iListOfPooledObjects.remove(0); // fetch from beginning
}
protected void returnPooledObject(Object o)
{
iListOfPooledObjects.add(o); // return at end
}
All handling is moved out to the abstract parent class, so the code
above really is all there is to it. The stack implementation looks like
this:
protected Stack iListOfPooledObjects = new Stack();
protected Object obtainPooledObject()
{
return iListOfPooledObjects.pop();
}
protected void returnPooledObject(Object o)
{
iListOfPooledObjects.push(o);
}
And similarly the priority pool will use a sorted collection with a
comparator.
> It's non-trivial to accept code that wasn't written by Apache members
> because of licensing issues but I'm happy to look and learn from a
> good idea when I see one.
Interesting. I didn't know that. I'll put it up once I add the priority
pool. An issue you mean a "bug" report in the commons area?
Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [POOL] a conceptual implementation error?
Posted by Sandy McArthur <sa...@gmail.com>.
On 11/21/06, Tom <tb...@tbee.org> wrote:
> I have a pool of 4 services. Every time a task needs to be performed, I
> borrow one service from the pool, have it execute the task, and return
> it. This seems to me as being a sensible pool.
Okay, I think I now have an idea of what you are trying to achieve. In
terms of Pool you would want a pool with a maxActive setting set to 4
and make sure there is no minIdle or idle object evictor or anything
else that could cause the pool to loose poolable objects.
But, I'm not sure a Pool is the best fit for what you want. If you
aren't strongly attached to a Stack ordering (LIFO) then I'd suggest a
BlockingQueue (FIFO) from util.concurrent. If you cannot use Java 1.5
then I'm pretty sure backport-util-concurrent will be able to help you
out. Simply stuff a ArrayBlockingQueue with your 4 service instances
and then take() and put() services. That will probably be faster than
the existing pools too. If you need to activate/passivate services you
may want to write a wrapper for this queue. It wouldn't be hard to
make that wrapper implement ObjectPool either.
http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/BlockingQueue.html
http://dcl.mathcs.emory.edu/util/backport-util-concurrent/
> Since using a factory is preferred, I have a collection based factory in
> which makeObject removes one from the collection and hands it over to
> the pool. There is no lazy creation here. When requesting a 5th service
> in this scenario, Apache's Stack makes the CPU go to 100%
I agree there shouldn't be an infinite loop if it can be avoided. I'll
make sure this gets fixed for the next release.
> I have tried to clearly explain why I feel there is a conceptual bug (I
> feel an implementation should not be able to go into an endless loop
> this easily). I understand that the currently implementation is
> perfectly suited for infinite (lazily create) pools. I have also shown
> IMHO that when the implementation is used for non infinite pools (and
> they do exist), they are buggy.
Pool assumes that poolable objects could become invalid over time. If
makeObject cannot create objects as needed, the the pool would run
dry.
> So, there is no pun intended here, and the current implementation has
> worked for our infinite pools perfectly for years. But I need an finite
> pool and have implemented both a round robin and a stack that works both
> in the lazy create and in the finite mode.
If you want to share those implementations, create an issue and attach
the sources. I cannot promise it will be included in the official
package but we can point others there if they have similar needs.
> It is up to the Apache
> developers to check and see if there is merit to my point and if you are
> interested in this alternative and would like to take a peek at the
> source. Or not. For me, my problem is solved.
It's non-trivial to accept code that wasn't written by Apache members
because of licensing issues but I'm happy to look and learn from a
good idea when I see one.
--
Sandy McArthur
"He who dares not offend cannot be honest."
- Thomas Paine
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [POOL] a conceptual implementation error?
Posted by Tom <tb...@tbee.org>.
> This is a nonsensical PoolableObjectFactory.
> Pools are designed so that they lazily create poolable objects as
> needed via the makeObject method.
> Using a pool without a PoolableObjectFactory is *strongly
> discouraged*.
I have a pool of 4 services. Every time a task needs to be performed, I
borrow one service from the pool, have it execute the task, and return
it. This seems to me as being a sensible pool.
Since using a factory is preferred, I have a collection based factory in
which makeObject removes one from the collection and hands it over to
the pool. There is no lazy creation here. When requesting a 5th service
in this scenario, Apache's Stack makes the CPU go to 100%, Generic
returns null. This is just as it is, I am sorry or maybe show me how to
implement this better. Preferred behaviour would be that the process
waits until a service becomes available.
I have tried to clearly explain why I feel there is a conceptual bug (I
feel an implementation should not be able to go into an endless loop
this easily). I understand that the currently implementation is
perfectly suited for infinite (lazily create) pools. I have also shown
IMHO that when the implementation is used for non infinite pools (and
they do exist), they are buggy.
So, there is no pun intended here, and the current implementation has
worked for our infinite pools perfectly for years. But I need an finite
pool and have implemented both a round robin and a stack that works both
in the lazy create and in the finite mode. It is up to the Apache
developers to check and see if there is merit to my point and if you are
interested in this alternative and would like to take a peek at the
source. Or not. For me, my problem is solved.
Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [POOL] a conceptual implementation error?
Posted by Sandy McArthur <sa...@apache.org>.
On 11/20/06, Tom <tb...@tbee.org> wrote:
> >> Currently the StackedObjectPool goes into a very CPU consuming
> >> busy-waiting loop
> >
> > If you can create an issue with some sample code that pegs the CPU
> > with StackObjectPool when it should be wait()ing I'd appreciate it
> > because that would be a bug that needs to be fixed by the next
> > release.
>
> It's simple to do: create PoolableObjectFactory that returns null on
> borrowObject and then create a StackObjectPool. The code below will have
> the CPU go to 100%. Enjoy :-)
[...]
> public class Test implements PoolableObjectFactory
> {
[...]
> public Object makeObject() throws Exception
> {
> System.out.println("makeObject");
> return null;
> }
This is a nonsensical PoolableObjectFactory. The makeObject has no
reason to return null. Either return a poolable object ready for use
or throw an Exception indicating why a poolable object could not be
created.
I'll admit the Javadocs for PoolableObjectFactory.makeObject does not
explicitly state that returning null from the method is prohibited but
that is implicit in it's affirmative statement that makeObject returns
"an instance that can be returned by the pool." See:
http://jakarta.apache.org/commons/pool/apidocs/org/apache/commons/pool/PoolableObjectFactory.html#makeObject()
> Now, I've been thinking in the car a bit more on this one. One could say
> that if the pool is infinite, it is not possible to initialize it with
> all possible values, therefore a factory is needed. In this case there
> is no BLOCK or FAIL amn GROW is default. If the pool is finite, then all
> possible values can be entered into the pooled set and there is no need
> for an unpooled set (factory). BLOCK and FAIL only need to be implement
> on the pooled set. This would closely match the current implementation,
Like any other library, it will be maximally effective if it you use
the library as it is intended. Pool provides a general purpose object
pooling api. The expectations are that the poolable objects are
interchangeable, reusable, and the lifecycle of poolable objects are
to be managed by the PoolableObjectFactory. Also to a lesser extent is
is expected that poolable objects are expensive to create and not
thread-safe else the performance reasons for using a pool are
mitigated. I'm not sure what you are trying to do but I think you are
trying hard to make a square peg fit in a round hole.
> but this would also mean that the pools should provide an easy mechanism
> for initial filling.
Pools are designed so that they lazily create poolable objects as
needed via the makeObject method. This helps elimiate a slow
application start up time and doesn't do more work that is actually
needed. If you feel you need to prefill your pool a utility method is
provided that does just that in PoolUtils.prefill:
http://jakarta.apache.org/commons/pool/apidocs/org/apache/commons/pool/PoolUtils.html#prefill(org.apache.commons.pool.ObjectPool,%20int)
> So:
> - infinite = factory, GROW, no BLOCK / FAIL, maxActive, maxIdle, etc
> - finite = no factory, initial filling, no GROW, BLOCK/FAIL, no
> maxActive, no maxIdle, etc
Using a pool without a PoolableObjectFactory is *strongly
discouraged*. It barely works only for backwards compatibility
reasons. Using a pool without a PoolableObjectFactory is prohibited in
the new pool implementations that will be available in the release of
Pool 2.0.
> I have written an alternative implementations using the same ObjectPool
> and PoolableObjectFactory interfaces.
If one of the provided pool implementations doesn't meet your needs
then writing your own implementation is what you should do.
--
Sandy McArthur
"He who dares not offend cannot be honest."
- Thomas Paine
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [POOL] a conceptual implementation error?
Posted by Tom <tb...@tbee.org>.
> What version of Pool are you using? Please make sure you are using
> Pool 1.3 or a nightly build that will likely become Pool 2.0
> semi-soon: http://jakarta.apache.org/commons/pool/downloads.html
1.3
>> Currently the StackedObjectPool goes into a very CPU consuming
>> busy-waiting loop
>
> If you can create an issue with some sample code that pegs the CPU
> with StackObjectPool when it should be wait()ing I'd appreciate it
> because that would be a bug that needs to be fixed by the next
> release.
It's simple to do: create PoolableObjectFactory that returns null on
borrowObject and then create a StackObjectPool. The code below will have
the CPU go to 100%. Enjoy :-)
import org.apache.commons.pool.PoolableObjectFactory;
import org.apache.commons.pool.impl.StackObjectPool;
public class Test implements PoolableObjectFactory
{
public void activateObject(Object arg0) throws Exception
{
}
public void destroyObject(Object arg0) throws Exception
{
}
public Object makeObject() throws Exception
{
System.out.println("makeObject");
return null;
}
public void passivateObject(Object arg0) throws Exception
{
}
public boolean validateObject(Object arg0)
{
return true;
}
public static void main(String[] args)
{
try
{
Test lTest = new Test();
StackObjectPool lStackObjectPool = new StackObjectPool(lTest);
lStackObjectPool.borrowObject();
}
catch (Throwable t) {t.printStackTrace(); }
}
}
Now, I've been thinking in the car a bit more on this one. One could say
that if the pool is infinite, it is not possible to initialize it with
all possible values, therefore a factory is needed. In this case there
is no BLOCK or FAIL amn GROW is default. If the pool is finite, then all
possible values can be entered into the pooled set and there is no need
for an unpooled set (factory). BLOCK and FAIL only need to be implement
on the pooled set. This would closely match the current implementation,
but this would also mean that the pools should provide an easy mechanism
for initial filling.
So:
- infinite = factory, GROW, no BLOCK / FAIL, maxActive, maxIdle, etc
- finite = no factory, initial filling, no GROW, BLOCK/FAIL, no
maxActive, no maxIdle, etc
I have written an alternative implementations using the same ObjectPool
and PoolableObjectFactory interfaces.
Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [POOL] a conceptual implementation error?
Posted by Sandy McArthur <sa...@apache.org>.
Tom,
> the GenericObjectPool just returns null.
What version of Pool are you using? Please make sure you are using
Pool 1.3 or a nightly build that will likely become Pool 2.0
semi-soon: http://jakarta.apache.org/commons/pool/downloads.html
One of the bigger clean ups recently is that the provided pool
implementations do not return null in lieu of no poolable object being
available. Instead the pools will throw a NoSuchElementException.
> Currently the StackedObjectPool goes into a very CPU consuming busy-waiting loop
If you can create an issue with some sample code that pegs the CPU
with StackObjectPool when it should be wait()ing I'd appreciate it
because that would be a bug that needs to be fixed by the next
release.
On 11/20/06, Tom <tb...@tbee.org> wrote:
> Bear with me on this one...
>
> The pool implementation splits pools into two parts: a provider of new
> objects (PoolableObjectFactory) and a pooling behaviour (Pool). This is
> a good idea, since this allows separation of content and behaviour: the
> same source of poolable objects can be used in a round-robin or stacked
> behaviour.
>
> Now, in the current implementation, when the pool runs dry, the active
> pooling behaviour can do one of three things: wait, create a new one, or
> fail. For example: GenericObjectPool has settings to specify what
> behaviour should be used: WHEN_EXHAUSTED_BLOCK, WHEN_EXHAUSTED_GROW,
> WHEN_EXHAUSTED_FAIL. Behaviour 1 and 3 are straight forward, but "GROW"
> is what I am having problems with.
>
> If we consider GROW behaviour in the current implementation, we can look
> at this as two sets inside the pool: one set with actively pooled
> objects (those managed by the pool and on this pool the max active, etc
> applies) and a second set with so far unpooled objects. The unpooled
> objects may not even be instantiated, but theoretically they exists.
> When the pool runs dry in GROW behaviour and thus needs a new object,
> one object is moved from the unpooled set to the pooled set.
>
> I consider this two-set setup an implementation detail: conceptually the
> pool consist of both the actively pooled and unpooled objects, the user
> of the pool just request a new object and the pool then must figure out
> how to get one and whether or not to juggle between the sets.
>
> Now to my point: in the current implementation this GROW behaviour ONLY
> applies to the pooled objects set. An that is IMHO incorrect. What
> should happen is: if the pooled objects set has run dry must ALWAYS try
> to fetch from the unpooled set, only if the unpooled set also has run
> dry, then the pool must decided wheter to BLOCK or WAIT. So it does not
> matter if the pooled set ran dry, BOTH set must have run dry in order to
> BLOCK or WAIT. Currently the StackedObjectPool goes into a very CPU
> consuming busy-waiting loop, and the GenericObjectPool just acceps null
> as the value.
>
> As state above, the behaviour should be applied on the combined set, not
> just the pooled set. Initially it seems that the behaviour should be
> extended to the unpooled set, however if you consider this for a moment
> longer, it turns out that the GROW behavour is actually not needed.
> Given that the implementation detail of the pooled and unpooled sets is
> of no concern to the user of the pool, he has no business in defining if
> a pool should GROW. Either the pool is infinite (the factory can endless
> continue creating new objects) or it is not. If the pool is infinite,
> BLOCK and WAIT are of no concern. If the pool is not infinite and runs
> dry, the pool can either WAIT for an object to be returned or FAIL.
> There is no GROW.
>
> So, to summarize:
> - Pools handle the pooled object set according to specs (max active, min
> active, etc).
> - Pool always GROW, its is the factory that decides if there is a new
> object available or not.
> - If the factory says there is no more object (the pool ran dry), then
> the pool decides if it will WAIT or FAIL.
> - The pool may decide that certain pooled objects are returned to the
> unpool set (destroy), when for example the "max active" is overflow.
>
> Now, tell me if I see this wrong or not. As said before: currently the
> StackedObjectPool goes CPU mongol on a dry pool and the
> GenericObjectPool just returns null. I currently consider both
> implementations seriously bugged.
>
> Tom
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-user-help@jakarta.apache.org
>
>
--
Sandy McArthur
"He who dares not offend cannot be honest."
- Thomas Paine
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org