aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kevin Sweeney" <kevi...@apache.org>
Subject Re: Review Request 24078: Add an InstanceUpdater to fit into rolling update coordination in the scheduler.
Date Fri, 01 Aug 2014 22:16:35 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message