Bug 1031199

Summary: EJB backing cache's can generate large retention from cancelled tasks in its scheduled executor's DelayedWorkQueue
Product: [JBoss] JBoss Enterprise Application Platform 6 Reporter: Aaron Ogburn <aogburn>
Component: EJBAssignee: Paul Ferraro <paul.ferraro>
Status: CLOSED CURRENTRELEASE QA Contact: Jan Martiska <jmartisk>
Severity: unspecified Docs Contact: Russell Dickenson <rdickens>
Priority: unspecified    
Version: 6.2.0CC: kkhan, paul.ferraro, smumford
Target Milestone: DR0   
Target Release: EAP 6.3.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
The cache implementation for @Stateful EJBs in JBoss EAP 6 utilizes a scheduled executor for handling @StatefulTimeout logic. When a bean is accessed, it's previous timeout job is cancelled, and a new one is rescheduled when the invocation completes. By default, cancelling a task from an executor did not remove it from the queue. This caused a gradual memory leak, as cancelled tasks remain in the queue. In this release, the scheduled executor can be configured to remove the task from the queue upon cancellation. This avoids the memory leak.
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-06-28 15:27:45 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Aaron Ogburn 2013-11-15 21:19:33 UTC
Description of problem:

EJB backing cache's frequently cancel remove and passivation tasks with each access and replace them with new ones.  Per http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ScheduledThreadPoolExecutor.html cancelled tasks are not removed from queue until their scheduled delay passes.

So due to the java ScheduledThreadPoolExecutor's lazy cancelled task removal, this scheduled task cancellation and recreation model can potentially churn up quite a number of queued cancelled tasks sitting in the executor's DelayedWorkQueue.  With longer timeouts and frequent ejb access, this can generate substantial heap overhead.



Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1. configure long ejb timeout with the Nonpassivating or PassivatingBackingCache
2. put load on the ejb

Actual results:

Cancelled tasks can pile in the executor's DelayedWorkQueue.


Expected results:

Cancelled tasks are flushed out of the executor's DelayedWorkQueue more eagerly.


Additional info:

It should be pretty easy to help limit any such build up by calling purge() [1] on the scheduled executor.  Likely don't want to purge with each cancel, so perhaps a purge() could be called on a configurable time delay or after a configurable amount of cancels?

[1] http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/ThreadPoolExecutor.html#purge%28%29

Comment 1 JBoss JIRA Server 2013-11-18 22:42:54 UTC
Paul Ferraro <paul.ferraro> updated the status of jira WFLY-2521 to Coding In Progress

Comment 2 JBoss JIRA Server 2013-11-18 22:53:34 UTC
David Lloyd <david.lloyd> made a comment on jira WFLY-2521

Using STPE for this probably isn't a great fit as it just doesn't do well with lots of cancellations.  A simple sorted fast-access queue of some sort would probably work much better.

Comment 3 JBoss JIRA Server 2013-11-18 23:09:33 UTC
Paul Ferraro <paul.ferraro> made a comment on jira WFLY-2521

[~dmlloyd] That's a good point.  It looks like queue removal is an O(N) operation - so even setRemoveOnCancelPolicy(true) is not ideal.  I'll open a separate jira to address this.

Comment 4 JBoss JIRA Server 2013-11-18 23:21:35 UTC
Paul Ferraro <paul.ferraro> made a comment on jira WFLY-2521

See: https://issues.jboss.org/browse/WFLY-2534

Comment 5 Paul Ferraro 2013-11-19 17:18:53 UTC
https://github.com/jbossas/jboss-eap/pull/706

Comment 6 Aaron Ogburn 2013-11-19 19:20:46 UTC
Hey Paul,


Were you planning to add similar cancelled task removes in the NonPassivatingBackingCache's executor as well?


Thanks,

Aaron

Comment 7 Paul Ferraro 2013-11-20 15:07:07 UTC
Good catch.  I've updated the PR.

Comment 8 Jan Martiska 2014-02-24 13:21:49 UTC
Verified in 6.3.0.DR0.