You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2014/08/07 21:25:16 UTC
Review Request 24465: Add a one-way job update controller.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/
-----------------------------------------------------------
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Bugs: AURORA-613
https://issues.apache.org/jira/browse/AURORA-613
Repository: aurora
Description
-------
There are 3 levels to performing an update:
1. Move the job from state A to state B, roll back on failure
2. Take a job from state A to state B
3. Take an instance from state A to state B
This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/24465/diff/
Testing
-------
./gradlew build -Pq
OneWayJobUpdater has 100% instruction and branch coverage.
Thanks,
Bill Farner
Re: Review Request 24465: Add a one-way job update controller.
Posted by Bill Farner <wf...@apache.org>.
> On Aug. 8, 2014, 4:17 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java, line 41
> > <https://reviews.apache.org/r/24465/diff/2/?file=655800#file655800line41>
> >
> > Missing @param <K> comment.
Fixed.
> On Aug. 8, 2014, 4:17 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java, line 65
> > <https://reviews.apache.org/r/24465/diff/2/?file=655800#file655800line65>
> >
> > How would instance events propagate from here? From what I can see the caller would not have the instance state exposure, so I am assuming it's going to be another "instanceEventSink" constructor argument?
Presumably events will directly correlation with actions taken on tasks, so that suggests they will be translated from InstanceActions.
> On Aug. 8, 2014, 4:17 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java, lines 130-132
> > <https://reviews.apache.org/r/24465/diff/2/?file=655800#file655800line130>
> >
> > This method is a bit hard to follow given the presence of both update and instance state machine transitions. How about encapsulating instance transitions in helper methods? Would make the algorithm here more readable.
Done, pulled the job-level transition out.
> On Aug. 8, 2014, 4:17 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java, lines 170-171
> > <https://reviews.apache.org/r/24465/diff/2/?file=655800#file655800line170>
> >
> > This would make it impossible to do a failure-agnostic rollback we currently have in client. Are you suggesting the rollback would be a best effort aborting in case of maxFailedInstances reached?
There are a few areas that need to be revisited for rollback, which should not be major. Added a TODO.
> On Aug. 8, 2014, 4:17 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java, lines 237-245
> > <https://reviews.apache.org/r/24465/diff/2/?file=655800#file655800line237>
> >
> > I thought the thrift JobUpdateAction is what we would use here, no?
Actions will be a subset of this enum.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/#review50038
-----------------------------------------------------------
On Aug. 7, 2014, 11:11 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24465/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 11:11 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-613
> https://issues.apache.org/jira/browse/AURORA-613
>
>
> Repository: aurora
>
>
> Description
> -------
>
> There are 3 levels to performing an update:
>
> 1. Move the job from state A to state B, roll back on failure
> 2. Take a job from state A to state B
> 3. Take an instance from state A to state B
>
> This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
> src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
> src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/24465/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> OneWayJobUpdater has 100% instruction and branch coverage.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 24465: Add a one-way job update controller.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/#review50038
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87549>
Missing @param <K> comment.
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87553>
How would instance events propagate from here? From what I can see the caller would not have the instance state exposure, so I am assuming it's going to be another "instanceEventSink" constructor argument?
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87552>
This method is a bit hard to follow given the presence of both update and instance state machine transitions. How about encapsulating instance transitions in helper methods? Would make the algorithm here more readable.
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87554>
This would make it impossible to do a failure-agnostic rollback we currently have in client. Are you suggesting the rollback would be a best effort aborting in case of maxFailedInstances reached?
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87555>
I thought the thrift JobUpdateAction is what we would use here, no?
- Maxim Khutornenko
On Aug. 7, 2014, 11:11 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24465/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 11:11 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-613
> https://issues.apache.org/jira/browse/AURORA-613
>
>
> Repository: aurora
>
>
> Description
> -------
>
> There are 3 levels to performing an update:
>
> 1. Move the job from state A to state B, roll back on failure
> 2. Take a job from state A to state B
> 3. Take an instance from state A to state B
>
> This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
> src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
> src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/24465/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> OneWayJobUpdater has 100% instruction and branch coverage.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 24465: Add a one-way job update controller.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/
-----------------------------------------------------------
(Updated Aug. 11, 2014, 9:29 p.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Bugs: AURORA-613
https://issues.apache.org/jira/browse/AURORA-613
Repository: aurora
Description
-------
There are 3 levels to performing an update:
1. Move the job from state A to state B, roll back on failure
2. Take a job from state A to state B
3. Take an instance from state A to state B
This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/24465/diff/
Testing
-------
./gradlew build -Pq
OneWayJobUpdater has 100% instruction and branch coverage.
Thanks,
Bill Farner
Re: Review Request 24465: Add a one-way job update controller.
Posted by Bill Farner <wf...@apache.org>.
> On Aug. 11, 2014, 9:05 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java, lines 135-137
> > <https://reviews.apache.org/r/24465/diff/2-3/?file=655800#file655800line135>
> >
> > There is still job/instance state update mix here. What I was trying to propose is a set of action methods like:
> > ...
> > reEvaluateInstances();
> > beginUpdatingNewInstances();
> > computeJobUpdateStatus();
Took a stab at this. To be honest, i think it reduces readability, but we can revisit later if necessary.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/#review50232
-----------------------------------------------------------
On Aug. 11, 2014, 8:19 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24465/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2014, 8:19 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-613
> https://issues.apache.org/jira/browse/AURORA-613
>
>
> Repository: aurora
>
>
> Description
> -------
>
> There are 3 levels to performing an update:
>
> 1. Move the job from state A to state B, roll back on failure
> 2. Take a job from state A to state B
> 3. Take an instance from state A to state B
>
> This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
> src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
> src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/24465/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> OneWayJobUpdater has 100% instruction and branch coverage.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 24465: Add a one-way job update controller.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/#review50232
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87877>
There is still job/instance state update mix here. What I was trying to propose is a set of action methods like:
...
reEvaluateInstances();
beginUpdatingNewInstances();
computeJobUpdateStatus();
- Maxim Khutornenko
On Aug. 11, 2014, 8:19 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24465/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2014, 8:19 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-613
> https://issues.apache.org/jira/browse/AURORA-613
>
>
> Repository: aurora
>
>
> Description
> -------
>
> There are 3 levels to performing an update:
>
> 1. Move the job from state A to state B, roll back on failure
> 2. Take a job from state A to state B
> 3. Take an instance from state A to state B
>
> This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
> src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
> src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/24465/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> OneWayJobUpdater has 100% instruction and branch coverage.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 24465: Add a one-way job update controller.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/
-----------------------------------------------------------
(Updated Aug. 11, 2014, 8:19 p.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Bugs: AURORA-613
https://issues.apache.org/jira/browse/AURORA-613
Repository: aurora
Description
-------
There are 3 levels to performing an update:
1. Move the job from state A to state B, roll back on failure
2. Take a job from state A to state B
3. Take an instance from state A to state B
This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/24465/diff/
Testing
-------
./gradlew build -Pq
OneWayJobUpdater has 100% instruction and branch coverage.
Thanks,
Bill Farner
Re: Review Request 24465: Add a one-way job update controller.
Posted by Bill Farner <wf...@apache.org>.
> On Aug. 7, 2014, 11:24 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java, lines 17-18
> > <https://reviews.apache.org/r/24465/diff/2/?file=655801#file655801line17>
> >
> > It's unclear to me why <T> is needed here. Also, this description seems too abstract (to the point where I'm not sure what is being described here).
Yeah, i was concerned about it being so abstract that it's confusing. I'll try to re-word this doc.
<T> is nice since it proves that OneWayJobUpdater is not coupled to the state type (IScheduledTask), which has the nicer side-effect of testing using a simple type (string). I considered using Function instead, but didn't want the baggage of @Nullable outputs.
> On Aug. 7, 2014, 11:24 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java, lines 17-20
> > <https://reviews.apache.org/r/24465/diff/2/?file=655801#file655801line17>
> >
> > Not clear why <T> is needed here. Also, the description seems too abstract, maybe add an example of what this might be used for?
Assuming this is a dupe comment of the above.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/#review49969
-----------------------------------------------------------
On Aug. 7, 2014, 11:11 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24465/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 11:11 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-613
> https://issues.apache.org/jira/browse/AURORA-613
>
>
> Repository: aurora
>
>
> Description
> -------
>
> There are 3 levels to performing an update:
>
> 1. Move the job from state A to state B, roll back on failure
> 2. Take a job from state A to state B
> 3. Take an instance from state A to state B
>
> This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
> src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
> src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/24465/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> OneWayJobUpdater has 100% instruction and branch coverage.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 24465: Add a one-way job update controller.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/#review49969
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java
<https://reviews.apache.org/r/24465/#comment87428>
It's unclear to me why <T> is needed here. Also, this description seems too abstract (to the point where I'm not sure what is being described here).
src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java
<https://reviews.apache.org/r/24465/#comment87429>
Not clear why <T> is needed here. Also, the description seems too abstract, maybe add an example of what this might be used for?
- Kevin Sweeney
On Aug. 7, 2014, 4:11 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24465/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 4:11 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-613
> https://issues.apache.org/jira/browse/AURORA-613
>
>
> Repository: aurora
>
>
> Description
> -------
>
> There are 3 levels to performing an update:
>
> 1. Move the job from state A to state B, roll back on failure
> 2. Take a job from state A to state B
> 3. Take an instance from state A to state B
>
> This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
> src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
> src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/24465/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> OneWayJobUpdater has 100% instruction and branch coverage.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 24465: Add a one-way job update controller.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/
-----------------------------------------------------------
(Updated Aug. 7, 2014, 11:11 p.m.)
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Changes
-------
Small interface tweaks - made the key identifier type generic, collapsed the two evaluate functions into one.
Bugs: AURORA-613
https://issues.apache.org/jira/browse/AURORA-613
Repository: aurora
Description
-------
There are 3 levels to performing an update:
1. Move the job from state A to state B, roll back on failure
2. Take a job from state A to state B
3. Take an instance from state A to state B
This implements level 2. I made the OneWayJobUpdater generic, which actually simplified both implementation and testing, since it is only responsible for relaying that state down to level 3.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 7476d82e9691449fa968c5fc4c5af76837a5c9cf
src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80
src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/24465/diff/
Testing
-------
./gradlew build -Pq
OneWayJobUpdater has 100% instruction and branch coverage.
Thanks,
Bill Farner