> On 2012-02-27 21:12:35, Angelo K. Huang wrote:
> > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/XCommand.java,
> > line 331
> > <https://reviews.apache.org/r/4035/diff/2/?file=85862#file85862line331>
> >
> > there is no need for this condition check when you already check lock
> > != null.
> >
> > lock != null already means:
> > inInterrupt == false
> > getEntityKey != null
>
> Mohamed Battisha wrote:
> if the entity key is checked while acquiring lock as you requested, we
> can remove it here
> But, we have to check for inInterrupt, otherwise we can be in an endless
> loop while executing interrupts within interrupts
>
> Angelo K. Huang wrote:
> Before you enter this executeInterrupts(), you already checked lock !=
> null. So these two conditions check are redundant.
>
> Mohamed Battisha wrote:
> nope, you have to check if you are executing regular or interrupt
> commands. We should avoid executing interrupts within interrupts.
First you get the lock only when it is not in the Interrupt mode. Then,
executeInterrupts will only called when lock is not NULL. So in the function
executeInterrupts(), this inInterruptMode() will always be false.
==> if (isLockRequired() && !this.inInterruptMode()) {
Instrumentation.Cron acquireLockCron = new
Instrumentation.Cron();
acquireLockCron.start();
acquireLock();
acquireLockCron.stop();
instrumentation.addCron(INSTRUMENTATION_GROUP, getName() +
".acquireLock", acquireLockCron);
}
==> if (lock != null) {
this.executeInterrupts();
}
- Angelo K.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4035/#review5359
-----------------------------------------------------------
On 2012-03-02 23:03:35, Mohamed Battisha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4035/
> -----------------------------------------------------------
>
> (Updated 2012-03-02 23:03:35)
>
>
> Review request for oozie.
>
>
> Summary
> -------
>
> While executing an interrupt driven commands it may be executed from the map
> on one thread and executed from the queue at a different thread
> This will cause the following exception to be thrown:
> java.lang.IllegalStateException: CoordChangeXCommand already used.
>
> - Avoiding throwing exception in case of interrupts and synchronizing the
> changes in [used]
> - Avoiding executing interrupt in case of commands that doesn't need locks
> - Changing the debug message for execute interrupt to explicitly mentioning
> it is an interrupt command
>
>
> This addresses bug OOZIE-684.
> https://issues.apache.org/jira/browse/OOZIE-684
>
>
> Diffs
> -----
>
>
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/util/XCallable.java
> 1296506
>
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/command/TestCommand.java
> 1296506
>
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java
> 1296506
>
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallableQueueService.java
> 1296506
>
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/Command.java
> 1296506
>
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/XCommand.java
> 1296506
>
> Diff: https://reviews.apache.org/r/4035/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohamed
>
>
