You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Purshotam Shah <pu...@yahoo-inc.com> on 2014/08/26 02:30:09 UTC
Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/
-----------------------------------------------------------
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
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review51661
-----------------------------------------------------------
this is good cleanup and refactoring. Did cursory review to understand structural changes. Will review more carefully for any bugs introduced later today
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment90178>
if bundleActionStatus map has some action as RUNNINGWITHERROR, why are we setting bundle job to PAUSED?
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment90177>
typo in bottom
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment90179>
same typo
- Mona Chitnis
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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52627
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91481>
There is no change in behaviour. Previous StatusTransitService has same condition.
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91482>
It should be PrepRunning state.
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91483>
Yes, we first check the terminal case. SUSPENDED check will come after terminal check.
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91485>
We check if all action is in terminal state then, we mark job as terminated. SKIPPED is also a terminal state.
core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/24948/#comment91486>
Yes, previous StatusTransitService does the same.
Lets create a different JIRA for batching.
- Purshotam Shah
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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> On Sept. 9, 2014, 3:12 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>
> >
> > But this is executed only if condition bAction.getCoordId() == null. So the case you mention will not occur
It will. "condition bAction.getCoordId() == null" means that one of bundle coord submit has failed and we need to kill bundle job.
> On Sept. 9, 2014, 3:12 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>
> >
> > Then I think we should 'skip' loading in SKIPPED actions from DB. The idea of skipped is that its outcome would not make any difference. So if all other actions are terminal, you will mark parent coord/bundle as terminal. if some are non-terminal, it wont be. ok to optimize this from earlier code
It make difference.
Assume there are 3 coord actions.
1. failed.
2. Skipped
3. Skipped.
Current behavior, final status will be DONEWITHERROR.
If I skip "Skipped" action then, final status will be failed. Same with killed.
I think it's ok to not change existing behavior.
- Purshotam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52722
-----------------------------------------------------------
On Sept. 8, 2014, 9:15 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2014, 9:15 p.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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52722
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91699>
But this is executed only if condition bAction.getCoordId() == null. So the case you mention will not occur
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91703>
Then I think we should 'skip' loading in SKIPPED actions from DB. The idea of skipped is that its outcome would not make any difference. So if all other actions are terminal, you will mark parent coord/bundle as terminal. if some are non-terminal, it wont be. ok to optimize this from earlier code
- Mona Chitnis
On Sept. 8, 2014, 9:15 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2014, 9:15 p.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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52944
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92202>
update only if something has changed
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92216>
Remove saveToDB option.
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92209>
This method is not used anywhere
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92206>
hasTerminatedActions() - Same as bundle
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92213>
Should not be updating database in getJobStatus. Already being done in the update method. Remove this from here
- Rohini Palaniswamy
On Sept. 8, 2014, 9:15 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2014, 9:15 p.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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52782
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/BundleJobBean.java
<https://reviews.apache.org/r/24948/#comment91893>
1) GET_BUNDLE_IDS_BY_LAST_MODIFIED_TIME -> GET_BUNDLE_IDS_FOR_STATUS_TRANSIT
2) Either A or a through out. If any of the supported databases is case-sensitive it will be a problem.
3) select distinct instead of group by at the end
4) Nitpick. spaces before and after = in w.id=a.bundleId
core/src/main/java/org/apache/oozie/CoordinatorJobBean.java
<https://reviews.apache.org/r/24948/#comment91894>
1) GET_COORD_IDS_BY_LAST_MODIFIED_TIME -> GET_COORD_IDS_FOR_STATUS_TRANSIT
2) distinct instead of group by
3) join clause w.id = a.jobId is missing.
core/src/main/java/org/apache/oozie/ErrorCode.java
<https://reviews.apache.org/r/24948/#comment91896>
Coord status transit error.
core/src/main/java/org/apache/oozie/ErrorCode.java
<https://reviews.apache.org/r/24948/#comment91897>
Bundle status transit error
core/src/main/java/org/apache/oozie/command/StatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91904>
0L
core/src/main/java/org/apache/oozie/command/StatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91905>
jobStatus
core/src/main/java/org/apache/oozie/command/StatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92173>
Current code will update status even if there is no change
core/src/main/java/org/apache/oozie/command/StatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92194>
Though this method is very readable, understanding the full status transit logic is very hard and requires lot of hops between methods to read. Requires rework.
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92164>
&& bundleJob.getStatus() != Job.Status.KILLED needs to be removed
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92163>
Remove variable and log directly
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92165>
Can we get delete CoordJobGetPendingActionsCountJPAExecutor and change to CoordJobQueryExecutor.getSingleValue. Refer to SLASummaryQueryExecutor.getSingleValue().
core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/24948/#comment92178>
Get rid of these. Not actually used
core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/24948/#comment92179>
Get rid of the hashset
core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/24948/#comment92180>
merge this method into bundleTransit
core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/24948/#comment92181>
merge this method into coordTransit
core/src/test/java/org/apache/oozie/service/TestStatusTransitService.java
<https://reviews.apache.org/r/24948/#comment92184>
services = new Services();
services.init();
to avoid leaking services
core/src/test/java/org/apache/oozie/service/TestStatusTransitService.java
<https://reviews.apache.org/r/24948/#comment92186>
JobLock. Why do you need a separate thread and class for acquiring and releasing locks?
Can't you just do
LockToken lock = Services.get().get(MemoryLocksService.class).getWriteLock(jobId, 0);
.....
Perform operations
.....
lock.release();
in the test methods
- Rohini Palaniswamy
On Sept. 8, 2014, 9:15 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2014, 9:15 p.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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review53208
-----------------------------------------------------------
StatusTransitXCommand abstract class defined methods in the order in which they are called. Please order the methods in the implementing commands same as the abstract class StatusTransitXCommand for easy code readability.
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92707>
Change message as below to make it more clear when reading the logs. Also change error code to E1320
Bundle action coordId is null and bundle job is already in killed state. Ignoring.
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment92708>
complete the javadoc. Else precommit will show this as error
core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/24948/#comment92709>
Can you delete this class?
- Rohini Palaniswamy
On Sept. 11, 2014, 1:42 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2014, 1:42 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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review53770
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/CoordinatorJobBean.java
<https://reviews.apache.org/r/24948/#comment93578>
This is not from your patch. But )}) does not look right. Can you check which patch introduced this and fix this as well?
- Rohini Palaniswamy
On Sept. 17, 2014, 10:56 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2014, 10:56 p.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 963497d
> core/src/main/java/org/apache/oozie/BundleJobBean.java 76e76b7
> core/src/main/java/org/apache/oozie/CoordinatorActionBean.java cc5596b
> core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 71a9ab4
> core/src/main/java/org/apache/oozie/ErrorCode.java f9f88f4
> 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 ce7473d
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionQueryExecutor.java 0aee0e4
> core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetPendingActionsCountJPAExecutor.java c96e5d9
> core/src/main/java/org/apache/oozie/executor/jpa/CoordJobQueryExecutor.java 2c9e00e
> core/src/main/java/org/apache/oozie/service/StatusTransitService.java 0d549a0
> core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetPendingActionsCountJPAExecutor.java 5b62fdf
> core/src/test/java/org/apache/oozie/service/TestStatusTransitService.java 1f9e76a
>
> Diff: https://reviews.apache.org/r/24948/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review53768
-----------------------------------------------------------
Ship it!
Ship It!
- Rohini Palaniswamy
On Sept. 17, 2014, 10:56 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2014, 10:56 p.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 963497d
> core/src/main/java/org/apache/oozie/BundleJobBean.java 76e76b7
> core/src/main/java/org/apache/oozie/CoordinatorActionBean.java cc5596b
> core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 71a9ab4
> core/src/main/java/org/apache/oozie/ErrorCode.java f9f88f4
> 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 ce7473d
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionQueryExecutor.java 0aee0e4
> core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetPendingActionsCountJPAExecutor.java c96e5d9
> core/src/main/java/org/apache/oozie/executor/jpa/CoordJobQueryExecutor.java 2c9e00e
> core/src/main/java/org/apache/oozie/service/StatusTransitService.java 0d549a0
> core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetPendingActionsCountJPAExecutor.java 5b62fdf
> core/src/test/java/org/apache/oozie/service/TestStatusTransitService.java 1f9e76a
>
> Diff: https://reviews.apache.org/r/24948/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/
-----------------------------------------------------------
(Updated Sept. 17, 2014, 10:56 p.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 (updated)
-----
core/src/main/java/org/apache/oozie/BundleActionBean.java 963497d
core/src/main/java/org/apache/oozie/BundleJobBean.java 76e76b7
core/src/main/java/org/apache/oozie/CoordinatorActionBean.java cc5596b
core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 71a9ab4
core/src/main/java/org/apache/oozie/ErrorCode.java f9f88f4
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 ce7473d
core/src/main/java/org/apache/oozie/executor/jpa/CoordActionQueryExecutor.java 0aee0e4
core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetPendingActionsCountJPAExecutor.java c96e5d9
core/src/main/java/org/apache/oozie/executor/jpa/CoordJobQueryExecutor.java 2c9e00e
core/src/main/java/org/apache/oozie/service/StatusTransitService.java 0d549a0
core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetPendingActionsCountJPAExecutor.java 5b62fdf
core/src/test/java/org/apache/oozie/service/TestStatusTransitService.java 1f9e76a
Diff: https://reviews.apache.org/r/24948/diff/
Testing
-------
UTC
Thanks,
Purshotam Shah
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/
-----------------------------------------------------------
(Updated Sept. 11, 2014, 1:42 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 (updated)
-----
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
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52789
-----------------------------------------------------------
Ship it!
Looks good now. Thanks for the 2 clarifications above.
- Mona Chitnis
On Sept. 8, 2014, 9:15 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2014, 9:15 p.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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/
-----------------------------------------------------------
(Updated Sept. 8, 2014, 9:15 p.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 (updated)
-----
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
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> 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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Mona Chitnis <mo...@yahoo.in>.
> 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?
>
> Purshotam Shah wrote:
> Yes, BundlePauseXCommand only set bundle status to pause. Bundle status can still be in running state.
Ok. I just checked that BundlePauseXCommand and CoordPauseXCommand have empty implementations of pauseChildren()
@Override
public void pauseChildren() throws CommandException {
// TODO - need revisit when revisiting coord job status redesign;
}
- Mona
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52346
-----------------------------------------------------------
On Sept. 8, 2014, 9:15 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24948/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2014, 9:15 p.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
>
>
Re: Review Request 24948: OOZIE-1940 StatusTransitService has race
condition
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24948/#review52346
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91109>
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
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91115>
related question, is this situation possible? - job status is PAUSED || PWE, and bundle action status is RWE?
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91116>
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
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91118>
I dont understand this. Are the other bundle actions in Running? Then why is status Prep and not Running?
core/src/main/java/org/apache/oozie/command/bundle/BundleStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91117>
why is getPrepStatus calling getRunningStatus?
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91119>
same comment as BundleStatusTransitX, just make sure whether SuspendedWithError is the right status here
core/src/main/java/org/apache/oozie/command/coord/CoordStatusTransitXCommand.java
<https://reviews.apache.org/r/24948/#comment91120>
why dont we filter out the SKIPPED actions altogether for status transit processing?
core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/24948/#comment91122>
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
- Mona Chitnis
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
>
>