Re: [Excalibur] DefaultPool: Could not create enough components to service your request

[Berin Loritsch <bl...@apache.org>]

tom.klaasen@the-ecorp.com wrote:
> 
> Hi,
> 
> The error mentioned in the subject line has been reported several times on
> this list.
> 
> Now, when putting load on Cocoon 2, I got the same error. And I found the
> cause.

Excellent!  I have been looking for this for a while.  I don't really have the
tools available to find these things (nor the time to do an exhaustive test).

> The problem is in this piece of code:
> 
> public Poolable get() throws Exception
> <snip/>
>   try
>      {
>       m_mutex.lock();
>       if( m_ready.size() == 0 )
>         {
>          if( this instanceof Resizable )
>            {
>             m_mutex.unlock();
>             ((Resizable)this).grow( m_controller.grow() );  //**
>             m_mutex.lock();
>             if ( m_ready.size() > 0 )
>                    {
>                     obj = (Poolable) m_ready.remove( 0 );
>                    }
>                 else
>                    {<etc/>
> 
> If this code is entered by thread1, and m_ready.size()==0, it tries to
> increase the pool and go on. No problem. But if thread1 is preempted at the
> position marked by //** by thread2, thread2 will find a m_ready.size() of
> 1, takes that instance out of the pool and sets m_ready.size() back to 0.
> When thread1 starts executing again, it will find no more instances in the
> pool and exit with an exception.

I understand completely.  I will apply your patch as soon as possible.

> (I hope I make myself a bit clear)
> 
> Now, if I'm trying to get something out of an SoftLimitingResourcePool, I
> would expect to always get an object. With the code provided here, this
> does not happen. The solution would be to put the grow() statement in a
> while loop:
> 
> while (m_ready.size() == 0) {
>    m_mutex.unlock();
>    ((Resizable)this).grow( m_controller.grow() );
>    m_mutex.lock();
> }
> 
> This will guarantee you that you have an instance available when you get
> out of the loop.
> 
> I tried this solution. And I got a deadlock. The problem was that for
> HardLimitingResourcePools, it might not be possible to acquire something
> out of the pool. And if some threads that have one instance out of the
> pool, are requiring another one, they will all be waiting on each other
> (thus a deadlock). So for HardResourceLimitingPools, we should throw an
> exception, and let the calling method decide what to do. In that way, the
> calling method should try to acquire all necessary resources before trying
> to use them (in a loop, each time releasing all resource that it has and
> waiting for some time.)

That is an issue with the HardLimitingResourcePool.  I tried using it with
the ComponentManager--but with improperly sized pools it makes Cocoon break
with deadlocks.  This puts more of an emphasis on configuring than I like
for a system that is supposed to just "work".

> So I came up with attached patch. I don't particularly like the
>         if (this instanceof HardResourceLimitingPool) {
> statement, but then again, neither do I like the
>         if( this instanceof Resizable )
> statement.

Instead of the grow() method, we can use the newPoolable() method (which
does not require giving up the lock).  I want to make the pools actively
managed--but I haven't figured out a clean way of doing it yet.

> I hope this makes it to excalibur (maybe in a slightly modified version),
> it would take out the racing condition that causes so much headaches.

I appreciate this very much.  If you have any more contributions you
want to submit, let me know.

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