Mailing-List: contact dev-help@geronimo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geronimo.apache.org
Received-SPF: pass (athena.apache.org: domain of manu.t.george@gmail.com
 designates 209.85.132.242 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=beta;
        h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references;
        b=W2g7DFdhhONkj94zkkarSV26KWTz5JyZgMd+V5JeZkiP8GFf9IJ5K72aM9whlKu9w3bz4D+aSJBkjo0mBXMmAjAJCRZdtTKfVwQMXy36n7QjRQdDmXgRFQ0uWrliDZlViGa11JyNIRZJkpTqlougoLRNxrYywKqG7AkHESLCoH8=
Message-ID: <466797bd0709040217j65d692fpb507127a6064e853@mail.gmail.com>
Date: Tue, 4 Sep 2007 14:47:00 +0530
From: "Manu George" <manu.t.george@gmail.com>
To: dev@geronimo.apache.org
Subject: Re: Thread Pool Deadlock
In-Reply-To: <466797bd0709022057p78a28d57l7e0dd2c77c2bb3b3@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <466797bd0709020846x5a759a30k6b568e334eb9254e@mail.gmail.com>
	 <C8FC72C5-DA5B-4643-98C7-2CEA13F42EAD@yahoo.com>
	 <466797bd0709021123w4f191ce8nae7e6e00b8d8211e@mail.gmail.com>
	 <EC7C9FBD-E601-462B-8FCF-96A1D8468FFE@yahoo.com>
	 <466797bd0709022057p78a28d57l7e0dd2c77c2bb3b3@mail.gmail.com>

Hi David,
          I was able to get sequential message processing in MDBs
using the following activation config properties

<activation-config>
<activation-config-property>
<activation-config-property-name>maxSessions</activation-config-property-name>
<activation-config-property-value>1</activation-config-property-value>
</activation-config-property>
<activation-config-property>
<activation-config-property-name>maxMessagesPerSessions
</activation-config-property-name>
<activation-config-property-value>1</activation-config-property-value>
</activation-config-property>
</activation-config>

got from the following AMQ documentation.
http://activemq.apache.org/activation-spec-properties.html

So no changes were required for the Thread Pool implementation. But do
you think that the
RejectionHandler needs to be changed as per the api docs? Or is it
fine as it is ?

Thanks
Manu

On 9/3/07, Manu George <manu.t.george@gmail.com> wrote:
> Hi David,
>                  Comments Inline
>
> On 9/3/07, David Jencks <david_jencks@yahoo.com> wrote:
> >
> > On Sep 2, 2007, at 11:23 AM, Manu George wrote:
> >
> > > Hi David,
> > >            Thanks for the explanation.
> > > In case of waitWhenBlocked=true what will be the expected behaviour if
> > > I set the poolsize as 1? Currently on debugging I see that the calling
> > > thread gets parked and then never gets resumed if already 1 thread is
> > > executing. Any idea why would this be happening?
> >
> > I suspect that the first thread is submitting work which needs a
> > second thread, or for the first one to complete.  In other words it
> > won't work with waitWhenBlocked = true.
>
> Yeah thought so. But with a LinkedBlockingQueue this works with the a
> pool size of 1. provided I set the rejection handler to the
> CallerRunsPolicy. So I was thinking maybe its because of the
> RejectionHandler that this is happening.
>
> > >
> > > Secondly the api docs say that we shouldn't do executor.getQueue
> > > ().put(r);
> > > Will this not create problems with unexpected behaviour?
> >
> > I didn't see this documentation, can you point me to it?  I don't see
> > how this could create unexpected problems, but I might have missed
> > something.
>
> Queue maintenance
>     Method getQueue() allows access to the work queue for purposes of
> monitoring and debugging. Use of this method for any other purpose is
> strongly discouraged.
>
> http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/ThreadPoolExecutor.html
>
> Another reference is OReilly Java Threads Page 192.
> Quote
> The getQueue() method returns the queue, but you should use that for
> debugging purposes only, don't execute methods directly on the queue
> or the internal workings of the thread pool become confused.
>
> Also in the same API doc its mentioned about direct handoffs
>
>  Direct handoffs. A good default choice for a work queue is a
> SynchronousQueue that hands off tasks to threads without otherwise
> holding them. Here, an attempt to queue a task will fail if no threads
> are immediately available to run it, so a new thread will be
> constructed. This policy avoids lockups when handling sets of requests
> that might have internal dependencies. Direct handoffs generally
> require unbounded maximumPoolSizes to avoid rejection of new submitted
> tasks. This in turn admits the possibility of unbounded thread growth
> when commands continue to arrive on average faster than they can be
> processed.
>
> So that was the reason I thought maybe we may need to change the queue
> implementation
>
> Would like to know your take on this
>
> >
> > thanks
> > david jencks
> > >
> > > Thanks
> > > Manu
> > >
> > >
> > > On 9/2/07, David Jencks <david_jencks@yahoo.com> wrote:
> > >> I don't think the current implementation is actually wrong under
> > >> normal use (where you just configure the gbean in xml and don't
> > >> change its settings at runtime).
> > >>
> > >> I think it would be better to set up the executor in its constructor
> > >> (keeping the waitWhenBlocked as a constructor parameter).  There's a
> > >> constructor for ThreadPoolExecutor we can use that lets us set
> > >> everything at once.
> > >>
> > >> I think the wait when blocked configuration is correct as it stands.
> > >> I suggest that using the LinkedBlockingQueue is appropriate when
> > >> waitWhenBlocked is false but not when its true.
> > >>
> > >> IMO the caller-runs policy is not appropriate for use in j2ca since
> > >> the work manager can notify you if the work is rejected.  Thus tying
> > >> up your own thread is not appropriate since it eliminates the
> > >> possibility of the caller taking corrective action.
> > >>
> > >> thanks
> > >> david jencks
> > >>
> > >> On Sep 2, 2007, at 8:46 AM, Manu George wrote:
> > >>
> > >>> Hi,
> > >>>         I was investigating why setting the resourceAdapter poolsize
> > >>> to 1 and using it in an Mdb for sequential message processing was
> > >>> failing and found that the org.apache.geronimo.pool.ThreadPool class
> > >>> in geronimo contains a ThreadPoolExecutor instance created with the
> > >>> constructor
> > >>>
> > >>> new ThreadPoolExecutor(
> > >>>             poolSize, // core size
> > >>>             poolSize, // max size
> > >>>             keepAliveTime, TimeUnit.MILLISECONDS,
> > >>>             new SynchronousQueue());
> > >>>
> > >>> The default behaviour is to reject the Runnables supplied when
> > >>> the no
> > >>> of active threads equals the pool size.
> > >>>
> > >>> Now the ThreadPool class has a setWaitWhenBlocked method which when
> > >>> called makes it wait. Setting  this makes the pool enter into a
> > >>> deadlock. The reason for this is the WaitWhenBlockedPolicy used to
> > >>> process rejection. In this class there is a
> > >>> executor.getQueue().put(r);
> > >>> Now the API docs mention that once you handoff the queue to a
> > >>> ThreadPoolExecutor we should not directly modify the queue as it may
> > >>> result in unpredictable behaviour. So this is a bug.
> > >>>
> > >>> As a solution for this what I did was
> > >>>
> > >>>  ThreadPoolExecutor p = new ThreadPoolExecutor(
> > >>>             poolSize, // core size
> > >>>             poolSize, // max size
> > >>>             keepAliveTime, TimeUnit.MILLISECONDS,
> > >>>             new LinkedBlockingQueue<Runnable>(queueSize));
> > >>>
> > >>> Now this results in upto queue size of Runnables that can be
> > >>> submitted
> > >>> of which poolsize will be the no of parallel executions. More than
> > >>> queue size will be rejected.
> > >>> We can also set the rejection handler to the CallerRunsPolicy
> > >>> when the
> > >>> quesize is exceeded and the waitWhenBlocked flag is true.
> > >>>
> > >>> This results in me getting the scenario working. I am not yet
> > >>> sure why
> > >>> the SynchronousQueue was used in this case, so thats why I
> > >>> changed the
> > >>> queuing strategy. So Is this an acceptable approach for fixing this
> > >>> issue?
> > >>> Secondly do we want a behaviour of rejection of work items when
> > >>> queue
> > >>> is full? Setting the CallerRunsPolicy actually results in graceful
> > >>> degradation when load exceeds capacity by increasing backlog at the
> > >>> TCP/IP layer. Would be happy if some of the experts would comment on
> > >>> this
> > >>>
> > >>> Thanks
> > >>> Manu
> > >>
> > >>
> >
> >
>
> Thanks
> Manu
>