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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 1, 2014, 2:50 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24078/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2014, 2:50 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim 
> Khutornenko.
> 
> 
> Bugs: AURORA-621
>     https://issues.apache.org/jira/browse/AURORA-621
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Note: this is one part of the epic AURORA-610, and is currently unwired code.
> 
> This implements the logic to initiate a change from a possibly-absent current 
> task state to a possibly-absent desired state.  I worked with Maxim on this, 
> and we started with a state machine approach.  After some more careful 
> thought, i pushed for a convergence function, which is implemented here.
> 
> There are a few invariants assumed/designed here, which i've called out in 
> the javadoc for evaluate().
> 
> The caller of this function will be responsible for implementing the 
> TaskController interface.  As for integration, this will mean:
>   - maintaining one InstanceUpdater for each in-flight instance update
>   - subscribing to task pubsub events and routing them to the appropriate 
> InstanceUpdater's evaluate()
>   - discarding terminal InstanceUpdaters (those which have called 
> TaskController#updateCompleted())
>   - scheduling delayed calls to evaluate() based on the job's update 
> parameters
> 
> My advice for reviewing is to first scan the evaluate() function (without 
> skipping out to helper functions), then dive into 
> handleActualAndDesiredPresent().  Think of evaluate() as something that may 
> be called arbitrarily, and attempts to gracefully move a task from the actual 
> state to the desired state.  Next, look at the unit test to see how the calls 
> should look to turn this function into an event-driven convergence function.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/TaskController.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24078/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq, 100% instruction and branch test coverage.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to