----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54439/#review158651 -----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java (line 153) <https://reviews.apache.org/r/54439/#comment229440> Can `event.getTimestamp() < pendingTs` ? src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java (line 190) <https://reviews.apache.org/r/54439/#comment229437> We can make `JobUpdateAction.INSTANCE_UPDATING` a parameter to the method and can use this same logic to track MTTRB (median time to rollback). src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java (line 199) <https://reviews.apache.org/r/54439/#comment229439> `taskEvent.getTimestamp() > start` ? Elongating the `timeFrame` might bring in cases where, task has following event lifecycle, ASSIGNED (t1) -> INSTANCE_UPDATING (t2) -> PENDING (t3) -> ASSIGNED (t4). MTTU => t1 - t2 < 0 - Santhosh Kumar Shanmugham On Dec. 8, 2016, 1:40 p.m., Joshua Cohen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54439/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2016, 1:40 p.m.) > > > Review request for Aurora, David McLaughlin, Mehrdad Nurolahzade, and > Santhosh Kumar Shanmugham. > > > Repository: aurora > > > Description > ------- > > The metric is calculated from the time of the `INSTANCE_UPDATING` event to > the subsequent `ASSIGNED` event for the task with the same instance id that > matches the desired task config from the update details. > > My original approach to this involved converting `GroupType` and > `AlgorithmType` from enums (which cannot be generic) to static classes > (which, of course, can). This allowed me to avoid unnecessarily passing > update details to the `calculate` method of `SlaAlgorithm` since it's ignored > in all but the one, new case. However, that ended up being a lot of churn, > and since it turns out we need both the task details and the update details > to calculate this metric, I went with the below approach. If anyone feels > strongly, I could go back to generics and create an container class that's > gives access to both the tasks and update details. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java > 9a56cda809fbbcb07e6dd12c7a0feb272542491d > src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java > 5d8d5bd8f705770979f284d26d2e932aabe707e5 > src/main/java/org/apache/aurora/scheduler/sla/SlaGroup.java > 6fbd4e962b3bb6eeb0831c810a321478fd52172c > src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java > 953b65f28a585375e36e305dea6f9f94f99abc93 > src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java > 2e719ac6b7aea86faa22deff2cc6b5f73135761c > src/test/java/org/apache/aurora/scheduler/sla/SlaModuleTest.java > 341e346e794c9cf9a2789b8799f38fff900ec9b3 > src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java > 78f440f7546de9ed6842cb51db02b3bddc9a74ff > > src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java > 21d26b3930ea965487b2dec48a48a98677ba022b > src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java > 43e32eede27bbf26363a3fd1ca34ffe6f8c01a73 > > src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java > 6d0e9bc6a8040393875d4f0a88e8db9d6926a88b > > Diff: https://reviews.apache.org/r/54439/diff/ > > > Testing > ------- > > ./gradlew build -Pq > e2e tests. > > > Thanks, > > Joshua Cohen > >