Re: Review Request 19888: Prepare to store UNKNOWN state.

2014-04-03 Thread Maxim Khutornenko

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

(Updated April 3, 2014, 5:31 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Rebased.


Bugs: AURORA-261
https://issues.apache.org/jira/browse/AURORA-261


Repository: aurora


Description
---

Before we start storing UNKNOWN (SANDBOX_DELETED) state need to prepare for a 
graceful rollback of that change.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
6622216c285eb1c25c7648769a7d7676e8b73bcc 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/19888/diff/


Testing
---

gradle clean build


Thanks,

Maxim Khutornenko



Re: Review Request 19888: Prepare to store UNKNOWN state.

2014-04-01 Thread Bill Farner

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


Isn't a StorageBackfill what you're after here?  Rewrite tasks in the UNKNOWN 
state to a more appropriate state?

- Bill Farner


On April 1, 2014, 9:52 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19888/
 ---
 
 (Updated April 1, 2014, 9:52 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-261
 https://issues.apache.org/jira/browse/AURORA-261
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Before we start storing UNKNOWN (SANDBOX_DELETED) state need to prepare for a 
 graceful rollback of that change.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
 003d475a1bd4ecc099d9a641fd239a8189f71cdb 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 d2becea60e5d7bb59a2e5adb66e10cd50f6b56f3 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 e77063a9c8e40e015ec264b151a7ed76f1c7f00f 
 
 Diff: https://reviews.apache.org/r/19888/diff/
 
 
 Testing
 ---
 
 gradle clean build
 gradle run
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 19888: Prepare to store UNKNOWN state.

2014-04-01 Thread Maxim Khutornenko


 On April 1, 2014, 10:43 p.m., Bill Farner wrote:
  Isn't a StorageBackfill what you're after here?  Rewrite tasks in the 
  UNKNOWN state to a more appropriate state?

The StorageBackfill is a much more invasive approach, which is not really 
required here. Besides, rewriting state would mess up the task events and show 
something like FINISHED - UNKNOWN - FINISHED. Trying to minimize the impact 
here.


- Maxim


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


On April 1, 2014, 9:52 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19888/
 ---
 
 (Updated April 1, 2014, 9:52 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-261
 https://issues.apache.org/jira/browse/AURORA-261
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Before we start storing UNKNOWN (SANDBOX_DELETED) state need to prepare for a 
 graceful rollback of that change.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
 003d475a1bd4ecc099d9a641fd239a8189f71cdb 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 d2becea60e5d7bb59a2e5adb66e10cd50f6b56f3 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 e77063a9c8e40e015ec264b151a7ed76f1c7f00f 
 
 Diff: https://reviews.apache.org/r/19888/diff/
 
 
 Testing
 ---
 
 gradle clean build
 gradle run
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 19888: Prepare to store UNKNOWN state.

2014-04-01 Thread Bill Farner


 On April 1, 2014, 10:43 p.m., Bill Farner wrote:
  Isn't a StorageBackfill what you're after here?  Rewrite tasks in the 
  UNKNOWN state to a more appropriate state?
 
 Maxim Khutornenko wrote:
 The StorageBackfill is a much more invasive approach, which is not really 
 required here. Besides, rewriting state would mess up the task events and 
 show something like FINISHED - UNKNOWN - FINISHED. Trying to minimize the 
 impact here.

I find changing state machine logic more invasive, actually.  This will impact 
other areas of the system.  For example, HistoryPruner will be unable to purge 
these tasks because UNKNOWN is not a TERMINAL_STATE.

However, with a backfill operation, you could flip a task in UNKNOWN back to 
the previous state, and delete the transition event to UNKNOWN (i.e. this 
doesn't need to go through the state machine).


- Bill


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


On April 1, 2014, 9:52 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19888/
 ---
 
 (Updated April 1, 2014, 9:52 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-261
 https://issues.apache.org/jira/browse/AURORA-261
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Before we start storing UNKNOWN (SANDBOX_DELETED) state need to prepare for a 
 graceful rollback of that change.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
 003d475a1bd4ecc099d9a641fd239a8189f71cdb 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 d2becea60e5d7bb59a2e5adb66e10cd50f6b56f3 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 e77063a9c8e40e015ec264b151a7ed76f1c7f00f 
 
 Diff: https://reviews.apache.org/r/19888/diff/
 
 
 Testing
 ---
 
 gradle clean build
 gradle run
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 19888: Prepare to store UNKNOWN state.

2014-04-01 Thread Maxim Khutornenko


 On April 1, 2014, 10:43 p.m., Bill Farner wrote:
  Isn't a StorageBackfill what you're after here?  Rewrite tasks in the 
  UNKNOWN state to a more appropriate state?
 
 Maxim Khutornenko wrote:
 The StorageBackfill is a much more invasive approach, which is not really 
 required here. Besides, rewriting state would mess up the task events and 
 show something like FINISHED - UNKNOWN - FINISHED. Trying to minimize the 
 impact here.
 
 Bill Farner wrote:
 I find changing state machine logic more invasive, actually.  This will 
 impact other areas of the system.  For example, HistoryPruner will be unable 
 to purge these tasks because UNKNOWN is not a TERMINAL_STATE.
 
 However, with a backfill operation, you could flip a task in UNKNOWN back 
 to the previous state, and delete the transition event to UNKNOWN (i.e. this 
 doesn't need to go through the state machine).

Rewriting the events would result in a broken UI sandbox link until the history 
pruner acts on it, whereas in my case UNKNOWN tasks were not shown in the UI. 
Anyway, I am ok with the backfill approach.


- Maxim


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


On April 1, 2014, 9:52 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19888/
 ---
 
 (Updated April 1, 2014, 9:52 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-261
 https://issues.apache.org/jira/browse/AURORA-261
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Before we start storing UNKNOWN (SANDBOX_DELETED) state need to prepare for a 
 graceful rollback of that change.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
 003d475a1bd4ecc099d9a641fd239a8189f71cdb 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 d2becea60e5d7bb59a2e5adb66e10cd50f6b56f3 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 e77063a9c8e40e015ec264b151a7ed76f1c7f00f 
 
 Diff: https://reviews.apache.org/r/19888/diff/
 
 
 Testing
 ---
 
 gradle clean build
 gradle run
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 19888: Prepare to store UNKNOWN state.

2014-04-01 Thread Maxim Khutornenko

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

(Updated April 2, 2014, 12:05 a.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Converting to StorageBackfill approach.


Bugs: AURORA-261
https://issues.apache.org/jira/browse/AURORA-261


Repository: aurora


Description
---

Before we start storing UNKNOWN (SANDBOX_DELETED) state need to prepare for a 
graceful rollback of that change.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
6622216c285eb1c25c7648769a7d7676e8b73bcc 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/19888/diff/


Testing (updated)
---

gradle clean build


Thanks,

Maxim Khutornenko



Re: Review Request 19888: Prepare to store UNKNOWN state.

2014-04-01 Thread Bill Farner

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

Ship it!



src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
https://reviews.apache.org/r/19888/#comment71585

use List#remove(int) here, it returns the removed element.


- Bill Farner


On April 2, 2014, 12:05 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19888/
 ---
 
 (Updated April 2, 2014, 12:05 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-261
 https://issues.apache.org/jira/browse/AURORA-261
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Before we start storing UNKNOWN (SANDBOX_DELETED) state need to prepare for a 
 graceful rollback of that change.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
 6622216c285eb1c25c7648769a7d7676e8b73bcc 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/19888/diff/
 
 
 Testing
 ---
 
 gradle clean build
 
 
 Thanks,
 
 Maxim Khutornenko