> On Sept. 4, 2014, 8:55 p.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java,
> >  line 81
> > <https://reviews.apache.org/r/24948/diff/1/?file=668667#file668667line81>
> >
> >     do we need to execute this synchronously? None of the counts here are 
> > affected by the outcome of this command. Can queue it to release lock faster

We need to kill bundle and coord asap. There may be chance the coord action 
have started and might be some processing. Since it's a faulty bundle, we don't 
want to start any processing.
This will be fixed in OOZIE-1863.


> On Sept. 4, 2014, 8:55 p.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java,
> >  line 177
> > <https://reviews.apache.org/r/24948/diff/1/?file=668667#file668667line177>
> >
> >     related question, is this situation possible? - job status is PAUSED || 
> > PWE, and bundle action status is RWE?

Yes, BundlePauseXCommand only set bundle status to pause. Bundle status can 
still be in running state.


> On Sept. 4, 2014, 8:55 p.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java,
> >  line 132
> > <https://reviews.apache.org/r/24948/diff/1/?file=668668#file668668line132>
> >
> >     same comment as BundleStatusTransitX, just make sure whether 
> > SuspendedWithError is the right status here

Yes, we first check the terminal case. SUSPENDED  check will come after 
terminal check.


> On Sept. 4, 2014, 8:55 p.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java,
> >  line 168
> > <https://reviews.apache.org/r/24948/diff/1/?file=668668#file668668line168>
> >
> >     why dont we filter out the SKIPPED actions altogether for status 
> > transit processing?

We check if all action is in terminal state then, we mark job as terminated. 
SKIPPED is also a terminal state. Previous StatusTransitService does same way.


> On Sept. 4, 2014, 8:55 p.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/service/StatusTransitService.java, line 
> > 160
> > <https://reviews.apache.org/r/24948/diff/1/?file=668672#file668672line160>
> >
> >     some form of batching done right away would be good, so we can execute 
> > bundle/coord update queries on multiple bundles/coords in a batched execute 
> > query. However then we wont be able to utilize the locking per job to have 
> > strong consistency. I guess ok to defer this until we have a better idea

Yes, previous StatusTransitService does the same.

Lets create a different JIRA for batching.


> On Sept. 4, 2014, 8:55 p.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java,
> >  line 248
> > <https://reviews.apache.org/r/24948/diff/1/?file=668667#file668667line248>
> >
> >     is this a change from current behavior? a mix of suspended, failed, 
> > killed, DWE, SPE = SPE? I'm not sure. Sounds reasonable to me but need to 
> > check if this could potentially be confusing to anyone

There is no change in behaviour. Previous StatusTransitService has same 
condition.


> On Sept. 4, 2014, 8:55 p.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java,
> >  line 267
> > <https://reviews.apache.org/r/24948/diff/1/?file=668667#file668667line267>
> >
> >     I dont understand this. Are the other bundle actions in Running? Then 
> > why is status Prep and not Running?

It should be PrepRunning state.


- Purshotam


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52346
-----------------------------------------------------------


On Aug. 26, 2014, 12:30 a.m., Purshotam Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2014, 12:30 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1940
>     https://issues.apache.org/jira/browse/OOZIE-1940
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> StatusTransitService has race condition
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/BundleActionBean.java 5d85a4d 
>   core/src/main/java/org/apache/oozie/BundleJobBean.java 0f1670a 
>   core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 795bf63 
>   core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 8fd53f1 
>   core/src/main/java/org/apache/oozie/ErrorCode.java 88a2c67 
>   core/src/main/java/org/apache/oozie/command/StatusTransitXCommand.java 
> e69de29 
>   
> core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
>  e69de29 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
>  e69de29 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleJobQueryExecutor.java 
> 36cd968 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionQueryExecutor.java
>  3008393 
>   core/src/main/java/org/apache/oozie/executor/jpa/CoordJobQueryExecutor.java 
> 04e6e29 
>   core/src/main/java/org/apache/oozie/service/StatusTransitService.java 
> 21ac25f 
>   core/src/test/java/org/apache/oozie/service/TestStatusTransitService.java 
> bb99138 
> 
> Diff: https://reviews.apache.org/r/24948/diff/
> 
> 
> Testing
> -------
> 
> UTC
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Reply via email to