> On March 26, 2014, 7:02 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java, line 130
> > <https://reviews.apache.org/r/19648/diff/1/?file=536631#file536631line130>
> >
> >     This API doesn't provide very good ergonomics.  (Frankly, it was 
> > already fairly poor with the existing Optional, my fault there.)  From the 
> > signature, it's not clear what, if anything, should happen when passing two 
> > absent ScheduleStatus arguments.  And i would classify the behavior as 
> > surprising.
> >     
> >     What made you decide to remove deleteTasks?

I was split on this but eventually decided to converge on changeState() to make 
it more explicit that the deletion goes through the state machine now. Happy to 
revert back to deleteTasks as I agree Optional makes it more painful to consume.


> On March 26, 2014, 7:02 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/Tasks.java, line 139
> > <https://reviews.apache.org/r/19648/diff/1/?file=536638#file536638line139>
> >
> >     Why is SANDBOX_DELETED not considered a terminal state?

Mostly to avoid possible regression. Added.


> On March 26, 2014, 7:02 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 278
> > <https://reviews.apache.org/r/19648/diff/1/?file=536648#file536648line278>
> >
> >     The statement about state reconciliation should probably go, i don't 
> > think it applies any more.

Dropped.


> On March 26, 2014, 7:02 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/MesosSchedulerImplTest.java, line 
> > 264
> > <https://reviews.apache.org/r/19648/diff/1/?file=536649#file536649line264>
> >
> >     testSandboxesDeleted?

Done.


> On March 26, 2014, 7:02 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/MesosSchedulerImplTest.java, line 
> > 265
> > <https://reviews.apache.org/r/19648/diff/1/?file=536649#file536649line265>
> >
> >     s/final // for all 3

Done.


- Maxim


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


On March 25, 2014, 11:44 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19648/
> -----------------------------------------------------------
> 
> (Updated March 25, 2014, 11:44 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Suman Karumuri, and Bill Farner.
> 
> 
> Bugs: AURORA-261
>     https://issues.apache.org/jira/browse/AURORA-261
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Part 2:
> - Renaming UNKNOWN to SANDBOX_DELETED;
> - Persisting SANDBOX_DELETED on legal transitions to ensure history is 
> retained;
> - Adding a new DELETED volatile state to guide task deletion on absent 
> ScheduleStatus value;
> - Dropping "delete" methods from SchedulerCore and StateManager interfaces to 
> ensure all task deletion goes through DELETED state change;
> - UI changes to report terminal state in case of GC deletion (screenshots 
> attached).
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 
> 772511f74fc12cf320f9eb4dcc857c58a3546aa1 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
> fd2644172e3814e8cf5f976753b07f6196368d71 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> 59f615c89ba1fad1656934da7dca6bd4ed741739 
>   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
> c3bf8c945b1f0b0923941660dd3eb57a5b614541 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 263235ce7c279f98920d7681f6162458be40593c 
>   src/main/java/org/apache/aurora/scheduler/async/TaskThrottler.java 
> 2334e7ff30843be4cbdd9086cbdafa61bb15ac94 
>   src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java 
> de79912bbd4825bba50e2d7be8eee5fd613b76d4 
>   src/main/java/org/apache/aurora/scheduler/base/Jobs.java 
> 4ac218b8f25be5e9e6b6243558af4258efe6e61a 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> 1e586c5ecc52ea32e50468942fd00a2d85463281 
>   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/MaintenanceController.java 
> 155fdd1e31bf52b61aab0ebbffb5264af644a38c 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 
> 7a13a8b9e8c9b7a767ec4bc4201e97de17809b82 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java 
> 3ca4ee529aaf3491118da216116b19d7c6a49d09 
>   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/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/UserTaskLauncherTest.java 
> 44cb20ebda1fc368a2570229e8294a266e858f57 
>   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/async/PreemptorImplTest.java 
> 24a35bf201077572b7973c4e46a46204ee3cd9a0 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> ce03abc5248dbc95306c759a04828830e3057b81 
>   src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java 
> a539b5931ed8b3a4a3e252e92ca1644b0c51a7fc 
>   src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java 
> bd1f599f889f1390731f520dfed9a3087bcaf00b 
>   src/test/java/org/apache/aurora/scheduler/base/JobsTest.java 
> effe14bf36d967b64a30af77378f106b862944cd 
>   src/test/java/org/apache/aurora/scheduler/base/TasksTest.java 
> 102fe04f8d3a7142a0cd58251e16d31e8d4a433d 
>   
> src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
>  033ebd29a4e95914347c1f5f8ade73115585a26f 
>   
> src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
>  faa38855438d2b147d32a158b3ebcd58089a7fc5 
>   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
> 24949c043b16a35a7960ed0a40b79f0981179eae 
>   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
> e77063a9c8e40e015ec264b151a7ed76f1c7f00f 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> 05c6e8a1e2407f2822b3844c555d8995f1cd1d49 
> 
> Diff: https://reviews.apache.org/r/19648/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh 
> 
> 
> File Attachments
> ----------------
> 
> Finished
>   
> https://reviews.apache.org/media/uploaded/files/2014/03/25/c5dd85ec-14df-4744-a2f9-d3af089e6286__CompletedFinished.png
> Deleted
>   
> https://reviews.apache.org/media/uploaded/files/2014/03/25/5bd3f5ec-89f8-4c91-a034-499a8db4caf9__CompletedDeleted.png
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to