aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 20058: Preserving sandbox deleted task history (part 2: state machine).
Date Sat, 05 Apr 2014 23:42:52 GMT

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

Ship it!



src/main/java/org/apache/aurora/scheduler/base/Jobs.java
<https://reviews.apache.org/r/20058/#comment72144>

    A comment here about why SANDBOX_DELETED case is absent would be nice to tie in the context
from above.



src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java
<https://reviews.apache.org/r/20058/#comment72145>

    Drop the TODO in favor of a NOTE, since we will likely not fix this behavior here.



src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java
<https://reviews.apache.org/r/20058/#comment72146>

    s/it/this/



src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java
<https://reviews.apache.org/r/20058/#comment72147>

    Can you leave a TODO to re-evaluate this?  My gut tells me *DELETED should not be valid
transitions here.



src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java
<https://reviews.apache.org/r/20058/#comment72148>

    better doc is needed to explain what an absent status means
    
    a TODO to formalize the delete/absent with a delete() method would be really nice



src/main/python/apache/aurora/executor/gc_executor.py
<https://reviews.apache.org/r/20058/#comment72149>

    Is this needed?  the values have not changed, only a name, so previously-bogus values
are still bogus.



src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java
<https://reviews.apache.org/r/20058/#comment72156>

    This is a strange function (it was strange before, too), since only one status is provided.
 This signature is pretty clear, though:
    
      List<TaskEvent> makeTaskEvents(
          long startTs,
          ScheduleStatus status,
          ScheduleStatus... statuses)
    
    Can you try that out?



src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java
<https://reviews.apache.org/r/20058/#comment72159>

    Is this redundant to the exhaustive state transition test case?  Possibly ditto for other
new test cases in this file.


- Bill Farner


On April 5, 2014, 1:12 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20058/
> -----------------------------------------------------------
> 
> (Updated April 5, 2014, 1:12 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-261
>     https://issues.apache.org/jira/browse/AURORA-261
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Rolling in the SANDBOX_DELETED changes originally introduced in https://reviews.apache.org/r/19648/.

> Also dropping StorageBackfill rewriting of UNKNOWN.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 772511f74fc12cf320f9eb4dcc857c58a3546aa1

>   src/main/java/org/apache/aurora/scheduler/base/Jobs.java 4ac218b8f25be5e9e6b6243558af4258efe6e61a

>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 003d475a1bd4ecc099d9a641fd239a8189f71cdb

>   src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 2ccc6f367b9715a0abb3e0673069289ae4860087

>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 8e3bfd318309918d92cfb75312fefb025a0002e4

>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java 2f6800043839dbb586bc774ccccb13fdd16ee09c

>   src/main/java/org/apache/aurora/scheduler/state/StateManager.java 4249707b283da078321faaaed006de54519238bc

>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 0161fac838ff5166161acd2960058de6856b0e9c

>   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java d2becea60e5d7bb59a2e5adb66e10cd50f6b56f3

>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 9abab00bddd48e7495eb7fdb60d71ea05e2a862a

>   src/main/python/apache/aurora/executor/gc_executor.py 4a866b2d6041a592692812eb3472db744d21e194

>   src/main/resources/org/apache/aurora/scheduler/http/schedulerzjob.st 28b56671b2e825912a6427e609c2bbe1e7758e26

>   src/main/thrift/org/apache/aurora/gen/api.thrift c0618e4edebd6f282698abfd9bdc3c36fff16920

>   src/test/java/org/apache/aurora/scheduler/MesosSchedulerImplTest.java 534b2b62cc2ae0ae9baab0247c9679350f03042b

>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a4e9464f7d5d3f5a640b62557c3e29f2f1566985

>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 378ece964081a7cb475dc6edd9b19ed506f03ec8

>   src/test/java/org/apache/aurora/scheduler/base/JobsTest.java effe14bf36d967b64a30af77378f106b862944cd

>   src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 2006ddb7f83a8ad2a1ce6ff3a96faf3c0bd8d8e9

>   src/test/java/org/apache/aurora/scheduler/base/TasksTest.java 102fe04f8d3a7142a0cd58251e16d31e8d4a433d

>   src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java f834bfb8c2339585214b0512e7df98dc75f931c7

>   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 24949c043b16a35a7960ed0a40b79f0981179eae

>   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java e77063a9c8e40e015ec264b151a7ed76f1c7f00f

>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java c6e318cabde9aad801b2a546415808473c9aff86

>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 05c6e8a1e2407f2822b3844c555d8995f1cd1d49

> 
> Diff: https://reviews.apache.org/r/20058/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh 
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


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