> On 2012-01-05 21:04:45, Alejandro Abdelnur wrote: > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/XCommand.java, > > line 245 > > <https://reviews.apache.org/r/3030/diff/4/?file=66761#file66761line245> > > > > adding to the comment of 3 different keys > > > > the fact that the lock is on a different key that the interruption it > > is an issue as the outer lock may be for a different key that the command > > being processed in the interruption
nope, the interrupt is using the lock key ... only two keys (key and entityKey [lockKey]) > On 2012-01-05 21:04:45, Alejandro Abdelnur wrote: > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/XCommand.java, > > line 483 > > <https://reviews.apache.org/r/3030/diff/4/?file=66761#file66761line483> > > > > This adds a third key to the XCommand > > > > getKey() > > getLockKey() > > getEntityKey() > > > > the acquireLock is done with the getEntityKey(), the getLock() is used > > for interruptions exclusively. > > > > I see 2 issues here. > > > > 1* the naming is confusing > > 2* Do we really need 3 different keys? > > There are only two keys [key, entity key] in the XCommand getLockKey() is mainly returning the getEntityKey() The main goal is to make the entity key exposed in the XCallable interface and implement it in XCommand [this is why we introduce the getLockKey() method in the XCallable interface] I understand the naming may be confusing, I will think about a different approach to eliminate this confusion > On 2012-01-05 21:04:45, Alejandro Abdelnur wrote: > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/XCommand.java, > > line 492 > > <https://reviews.apache.org/r/3030/diff/4/?file=66761#file66761line492> > > > > no need to use 'this.' Yep, I understand but I just get used to put this before any private member. it make it easy for reading the code. it is a recommended standard for a lot of OO languages > On 2012-01-05 21:04:45, Alejandro Abdelnur wrote: > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleJobResumeXCommand.java, > > line 137 > > <https://reviews.apache.org/r/3030/diff/4/?file=66762#file66762line137> > > > > false change! (there are several of these through the patch) will revert this file .. no need to have it for only this change > On 2012-01-05 21:04:45, Alejandro Abdelnur wrote: > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundlePauseXCommand.java, > > line 45 > > <https://reviews.apache.org/r/3030/diff/4/?file=66763#file66763line45> > > > > is it posible/valid no to have a bundleJOb here? > > > > same Q for similar changes in other commands nope, for this case it is not in some other changes, during my unit-test i faced several null pointer exceptions. i just added them all over for consistency I can open another JIRA to address this issue in a more broad way - Mohamed ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3030/#review4211 ----------------------------------------------------------- On 2012-01-05 20:12:58, Mohamed Battisha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3030/ > ----------------------------------------------------------- > > (Updated 2012-01-05 20:12:58) > > > Review request for oozie. > > > Summary > ------- > > Oozie continues to materialize new actions after end date modification. The > main issue is related how Oozie materialized the actions based on a FIFO > priority queue. Changing the status of a bundle/coordinator job should take > higher priority than executing this job. > > The main idea is to enable the queue to handle interruptions. Mainly, once > you finished what you are working on currently, you should focus on executing > this next action. > > The issue can be illustrated as follow: > > 1. Configure a pipeline to run for 1 hour > 2. Start the pipeline > 3. After it starts materializing new actions, change the end time (in my > example - to 10 minutes after the pipeline > starts) > 4. Monitor the coordinator apps - they will continue to materialize new > actions past the end time. > > > This addresses bug OOZIE-591. > https://issues.apache.org/jira/browse/OOZIE-591 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/Command.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/XCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleJobResumeXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundlePauseXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleRerunXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleStartXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleStatusUpdateXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleUnpauseXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionNotificationXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionReadyXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionTimeOutXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdateXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPauseXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordRerunXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordResumeXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSuspendXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordUnpauseXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/wf/ResumeXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/wf/WfEndXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallableQueueService.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/RecoveryService.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/util/XCallable.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/oozie-default.xml > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/command/TestCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java > 1209829 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java > 1209829 > > Diff: https://reviews.apache.org/r/3030/diff > > > Testing > ------- > > Regression for all the Commands in addition for specific testing for the > Interrupt Driven Map > > > Thanks, > > Mohamed > >
