Wow, that was quick. I'm quite shocked actually :)
You've definitely got the big picture.
On May 28, 2010, at 6:56 AM, Thiago Veronezi wrote:
> Hi David,
> thanks for the directions. I think thats a good task for me to have a global
> view of the system. I think its becoming more clear how to implement the
> fix. Please, verify it for me...
>
> ************************************
> add new class org.apache.openejb.jee.StatefulTimeoutConfig
>
> class StatefulTimeoutConfig {
> @XmlEnumValue("timeout") TIMEOUT,
> @XmlEnumValue("unit") UNIT;
> }
Note that @StatefulTimeout is a different annotation then @AccessTimeout -- see
spec for details. It has the exact xml schema details too.
We will have to support both, so feel free to add that JAXB item while your in
the code. Perhaps a "Timeout" class that could be reused by both.
Note also that <access-timeout> is a list as it's possible to specify the
access timeout for each individual method. I'm totally fine if we want to
ignore that in the in the first patch and come back and do it in a second
revision. It's already a lot to chew on and "per method" metadata is tougher
to pull through the system.
On naming conventions, we typically go with the xml element name with no added
suffix.
> ************************************
> TransactionType
>
> Add a new property (say statefulTimeout) to
> the org.apache.openejb.jee.SessionBean class (below)
>
> @XmlElement(name = "stateful-timeout")
> protected StatefulTimeoutConfig statefulTimeout;
>
> ************************************
> Add the new entry (on bold - same org.apache.openejb.jee.SessionBean class)
> to the "session-beanType" annotation
>
> @XmlType(name = "session-beanType", propOrder = {
> "descriptions",
> "displayNames",
> "icon",
> "ejbName",
> "mappedName",
> "home",
> "remote",
> "localHome",
> "local",
> "businessLocal",
> "businessRemote",
> "localBean",
> "serviceEndpoint",
> "ejbClass",
> "sessionType",
> "loadOnStartup",
> "timeoutMethod",
> "initMethod",
> "removeMethod",
> "transactionType",
> "concurrencyType",
> "aroundInvoke",
> "envEntry",
> "ejbRef",
> "ejbLocalRef",
> "serviceRef",
> "resourceRef",
> "resourceEnvRef",
> "messageDestinationRef",
> "persistenceContextRef",
> "persistenceUnitRef",
> "postConstruct",
> "preDestroy",
> "postActivate",
> "prePassivate",
> "securityRoleRef",
> "securityIdentity",
> "dependsOn",
> *"statefulTimeout"*
> })
Exactly. Ditto for "acccessTimeout".
> ************************************
> Create a new Class...
> class AccessTimeoutValue {
> long time;
> TimeUnit unit;
> }
>
> ************************************
> add new property to org.apache.openejb.core.CoreDeploymentInfo class
>
> private AccessTimeoutValue accessTimeoutValue;
>
> AccessTimeoutValue getAccessTimeoutValue() {
> return this.accessTimeoutValue;
> }
>
> void setAccessTimeoutValue(AccessTimeoutValue accessTimeoutValue) {
> this.accessTimeoutValue = accessTimeoutValue;
> }
For the CoreDeploymentInfo class we can use an existing class called
org.apache.openejb.util.Duration. It does some other fancy stuff, but
basically it's a long/TimeUnit tuple.
As noted above, feel free to ignore the fact that AccessTimeout can be done on
a per method basis. Eventually we'll need some map here, but it's fine to
ignore because I have some fancy ideas for what we should use as the default
when they haven't specified the AccessTimeout for a specific method.
Maybe better to get something basic done and checked in. I suspect the code
will be changing kind of fast with all the other EJB 3.1 work we have going on
and holding big changes can be hard. In fact, feel free to submit patches for
individual little parts of this while chunk of work. I tried to break it up
into sort of logical "bites" in our road map:
http://openejb.apache.org/ejb-31-roadmap.html
Feel free to attach patches to those individual jiras.
> ************************************
> Class org.apache.openejb.assembler.classic.EnterpriseBeanInfo
>
> add new property
> public AccessTimeoutValue accessTimeout;
>
> ************************************
>
> Class org.apache.openejb.config.EjbJarInfoBuilder (line 564)
>
> bean.accessTimeout = new AccessTimeoutValue(
> Long.valueof(s.geStatefulTimeout().TIMEOUT),
> Long.valueof(s.geStatefulTimeout().UNIT));
To match the Timeout class of the jaxb tree, we can make a TimeoutInfo class.
See OpenEjbConfigurationValidationTest for the rules we have in place.
> ************************************
> Class org.apache.openejb.core.stateful.StatefulContainer.
[...]
> Use...
> synchronized (instance) {
> *final AccessTimeoutValue accessTimeout =
> instance.deploymentInfo.getAccessTimeoutValue();*
>
> Lock currLock = instance.getLock();
> final boolean lockAcquired;
> if(accessTimeout == null) {
> // returns immediately true if the lock is available
> lockAcquired = currLock.tryLock();
> } else {
> // AccessTimeout annotation found.
> // Trying to get the lock within the specified period.
> try {
> lockAcquired = currLock.tryLock(accessTimeout.value(),
> accessTimeout.unit());
> } catch (InterruptedException e) {
> throw new ApplicationException("Unable to get lock.", e);
> }
> }
> // Did we acquire the lock to the current execution?
> if (!lockAcquired) {
> throw new ApplicationException(
> new ConcurrentAccessTimeoutException("Unable to get lock."));
> }
This is the part that is a little tricky. Basically all of that synchronized
block has to get rewritten. The "gotcha" is that if someone is waiting
(tryAquire(1, MINUTE)) inside a synchronized block, then all other threads
trying to access that instance will also have to wait just to enter the
synchronized block and get their chance to call "tryAquire", which if course
involves more waiting.
I'm not entirely sure we need the synchronized block anymore. At best we need
it after the lock aquire. That of course makes the "tryLock()" call already
happening a bit strange.
Will probably take some experimentation/testing to get it nailed down.
-David
