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 19648: Preserving sandbox deleted task history.
Date Wed, 26 Mar 2014 19:02:15 GMT

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


Biggest dislike is the new Optional argument.  I'd rather retain the explicit API for deletion,
probably all the way into TaskStateMachine.


src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java
<https://reviews.apache.org/r/19648/#comment70897>

    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?



src/main/java/org/apache/aurora/scheduler/base/Tasks.java
<https://reviews.apache.org/r/19648/#comment70896>

    Why is SANDBOX_DELETED not considered a terminal state?



src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/19648/#comment70903>

    The statement about state reconciliation should probably go, i don't think it applies
any more.



src/test/java/org/apache/aurora/scheduler/MesosSchedulerImplTest.java
<https://reviews.apache.org/r/19648/#comment70905>

    testSandboxesDeleted?



src/test/java/org/apache/aurora/scheduler/MesosSchedulerImplTest.java
<https://reviews.apache.org/r/19648/#comment70904>

    s/final // for all 3


- Bill Farner


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


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