> 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 195 > > <https://reviews.apache.org/r/4035/diff/2/?file=85862#file85862line195> > > > > we should not allow getEntityKey == null to get the lock. Please add > > condition check here too.
sure will add it. I was assuming the null checking should happen generically while getting the write lock > 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 263 > > <https://reviews.apache.org/r/4035/diff/2/?file=85862#file85862line263> > > > > if lock != null, isLockRequired() must be true. > > > > So you can remove isLockRequired(). Yep, good catch > 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 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 - Mohamed ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4035/#review5359 ----------------------------------------------------------- On 2012-02-24 19:44:14, Mohamed Battisha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4035/ > ----------------------------------------------------------- > > (Updated 2012-02-24 19:44:14) > > > 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/command/XCommand.java > 1293381 > > Diff: https://reviews.apache.org/r/4035/diff > > > Testing > ------- > > > Thanks, > > Mohamed > >
