You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by Thiago Veronezi <tv...@gmail.com> on 2010/06/01 23:02:06 UTC
Re: @AccessTimeout (was Re: unit test issue)
Hi, devs.
Thats just to say that I've uploaded a patch for the issues below (here:
https://issues.apache.org/jira/browse/OPENEJB-1144)
When you have time, please take a look on that for me.
tkx,
Thiago.
On Mon, May 31, 2010 at 3:38 PM, David Blevins <da...@visi.com>wrote:
>
> On May 31, 2010, at 5:13 AM, Thiago Veronezi wrote:
>
> > Hi, David!
> > I've just finished the coding and unit-testing for the ejb-jar stuff:
> > http://issues.apache.org/jira/browse/OPENEJB-1144 and
> > http://issues.apache.org/jira/browse/OPENEJB-1146.
>
> Great!
>
> > I'de like to create a
> > patch for that, but Im not sure how to do it.
> > Actually, what is a patch? Is it a screenshot of my code with no hidden
> svn
> > files for you (or the other devs) to verify commit? Where do I publish
> it?
> > tkx!
>
> Basically if you type 'svn diff' it will show all the lines of code that
> were changed. There's a corresponding program, called 'patch', that will
> read the patch file and edit all the code accordingly.
>
> So you can do something like:
>
> svn diff > OPENEJB-1144.patch
>
> And attach that OPENEJB-1144.patch to the OPENEJB-1144 JIRA issue.
>
> One patch for the two JIRAs is fine. No need to attach the same one twice.
>
> This page has a tiny bit more info:
>
> http://wiki.creativecommons.org/HOWTO_Patch
>
> -David
>
> >
> >
> > On Fri, May 28, 2010 at 3:44 PM, David Blevins <david.blevins@visi.com
> >wrote:
> >
> >> 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
> >>
> >>
>
>
Re: @AccessTimeout (was Re: unit test issue)
Posted by David Blevins <da...@visi.com>.
On Jun 2, 2010, at 7:49 AM, Thiago Veronezi wrote:
> A little update.
> I had forgot to change the AnnotationDeployer class to merge the deployment
> info from the xml file.
> The issue OPENEJB-1144 has now a new patch file. The newest one is the valid
> one.
Excellent! Applied that latest patch with the AnnotationDeployer changes.
Made some changes as well. Mainly merged the AccessTimeout/StatefulTimeout JAXB objects to a common Timeout class. As well created a TimeoutInfo object for use in the EnterpriseBeanInfo class.
Fantastic work, Thiago. You're really cooking!
-David
Re: @AccessTimeout (was Re: unit test issue)
Posted by Thiago Veronezi <tv...@gmail.com>.
A little update.
I had forgot to change the AnnotationDeployer class to merge the deployment
info from the xml file.
The issue OPENEJB-1144 has now a new patch file. The newest one is the valid
one.
tkx,
Thiago.
On Tue, Jun 1, 2010 at 5:02 PM, Thiago Veronezi <tv...@gmail.com> wrote:
> Hi, devs.
> Thats just to say that I've uploaded a patch for the issues below (here:
> https://issues.apache.org/jira/browse/OPENEJB-1144)
> When you have time, please take a look on that for me.
> tkx,
> Thiago.
>
>
> On Mon, May 31, 2010 at 3:38 PM, David Blevins <da...@visi.com>wrote:
>
>>
>> On May 31, 2010, at 5:13 AM, Thiago Veronezi wrote:
>>
>> > Hi, David!
>> > I've just finished the coding and unit-testing for the ejb-jar stuff:
>> > http://issues.apache.org/jira/browse/OPENEJB-1144 and
>> > http://issues.apache.org/jira/browse/OPENEJB-1146.
>>
>> Great!
>>
>> > I'de like to create a
>> > patch for that, but Im not sure how to do it.
>> > Actually, what is a patch? Is it a screenshot of my code with no hidden
>> svn
>> > files for you (or the other devs) to verify commit? Where do I publish
>> it?
>> > tkx!
>>
>> Basically if you type 'svn diff' it will show all the lines of code that
>> were changed. There's a corresponding program, called 'patch', that will
>> read the patch file and edit all the code accordingly.
>>
>> So you can do something like:
>>
>> svn diff > OPENEJB-1144.patch
>>
>> And attach that OPENEJB-1144.patch to the OPENEJB-1144 JIRA issue.
>>
>> One patch for the two JIRAs is fine. No need to attach the same one
>> twice.
>>
>> This page has a tiny bit more info:
>>
>> http://wiki.creativecommons.org/HOWTO_Patch
>>
>> -David
>>
>> >
>> >
>> > On Fri, May 28, 2010 at 3:44 PM, David Blevins <david.blevins@visi.com
>> >wrote:
>> >
>> >> 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
>> >>
>> >>
>>
>>
>