Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread David McLaughlin


 On Aug. 15, 2014, 6:02 p.m., Maxim Khutornenko wrote:
  src/main/thrift/org/apache/aurora/gen/api.thrift, line 558
  https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line558
 
  Rolling back assumes removing new (INSTANCE_REMOVED) and adding old 
  (INSTANCE_ADDED). Do we want to distinct those actions from the forward 
  roll?
 
 David McLaughlin wrote:
 INSTANCE_REMOVED is only used when you remove an instance because the new 
 instanceCount is less than previous one, it's not used if you're removing an 
 instance as part of updating it.
 
 Maxim Khutornenko wrote:
 I guess I did not see the JobUpdateAction as a post factum update story 
 teller. I thought it would be used more as a facilitator for internal actions 
 to be done (i.e. add_instance, kill_instance, and etc.) 
 
 I don't have a problem with this high level mapping as long as updater 
 would not rely on it to drive the state machine progress.

We should be careful either way not to leak internal state machines into the 
public API. But yes, I think definitely the purpose of instance events is to 
retell a story of what happened during this update.


- David


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


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread David McLaughlin


 On Aug. 16, 2014, 12:23 a.m., Maxim Khutornenko wrote:
  src/main/thrift/org/apache/aurora/gen/api.thrift, line 538
  https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line538
 
  What action is going to be used for an instance that failed to update 
  and the job rollback is disabled?

Sounds like we need another state for this. Do you have a suggestion? 
INSTANCE_UPDATE_FAILED ?


- David


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


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner


 On Aug. 16, 2014, 12:23 a.m., Maxim Khutornenko wrote:
  src/main/thrift/org/apache/aurora/gen/api.thrift, line 538
  https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line538
 
  What action is going to be used for an instance that failed to update 
  and the job rollback is disabled?
 
 David McLaughlin wrote:
 Sounds like we need another state for this. Do you have a suggestion? 
 INSTANCE_UPDATE_FAILED ?

INSTANCE_UPDATE_FAILED SGTM


- Bill


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


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner


 On Aug. 16, 2014, 12:23 a.m., Maxim Khutornenko wrote:
  src/main/thrift/org/apache/aurora/gen/api.thrift, line 538
  https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line538
 
  What action is going to be used for an instance that failed to update 
  and the job rollback is disabled?
 
 David McLaughlin wrote:
 Sounds like we need another state for this. Do you have a suggestion? 
 INSTANCE_UPDATE_FAILED ?
 
 Bill Farner wrote:
 INSTANCE_UPDATE_FAILED SGTM

And FWIW disabled rollback isn't the only case for this.  It can happen any 
time max_failed_instances  0.


- Bill


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


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Maxim Khutornenko


 On Aug. 16, 2014, 12:23 a.m., Maxim Khutornenko wrote:
  src/main/thrift/org/apache/aurora/gen/api.thrift, line 538
  https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line538
 
  What action is going to be used for an instance that failed to update 
  and the job rollback is disabled?
 
 David McLaughlin wrote:
 Sounds like we need another state for this. Do you have a suggestion? 
 INSTANCE_UPDATE_FAILED ?
 
 Bill Farner wrote:
 INSTANCE_UPDATE_FAILED SGTM
 
 Bill Farner wrote:
 And FWIW disabled rollback isn't the only case for this.  It can happen 
 any time max_failed_instances  0.

Do we need a separate INSTANCE_ROLLBACK_FAILED state to set apart a rollback 
failure? Otherwise, we may see two actions in the log: one from a forward 
failure and another one from a rollback failure.


- Maxim


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


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner


 On Aug. 16, 2014, 12:23 a.m., Maxim Khutornenko wrote:
  src/main/thrift/org/apache/aurora/gen/api.thrift, line 538
  https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line538
 
  What action is going to be used for an instance that failed to update 
  and the job rollback is disabled?
 
 David McLaughlin wrote:
 Sounds like we need another state for this. Do you have a suggestion? 
 INSTANCE_UPDATE_FAILED ?
 
 Bill Farner wrote:
 INSTANCE_UPDATE_FAILED SGTM
 
 Bill Farner wrote:
 And FWIW disabled rollback isn't the only case for this.  It can happen 
 any time max_failed_instances  0.
 
 Maxim Khutornenko wrote:
 Do we need a separate INSTANCE_ROLLBACK_FAILED state to set apart a 
 rollback failure? Otherwise, we may see two actions in the log: one from a 
 forward failure and another one from a rollback failure.

I'm ambivalent, and leave it up to what we want to do display-side.  I don't 
plan to have these entries influence the updater.


- Bill


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


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread David McLaughlin

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

(Updated Aug. 18, 2014, 5:49 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

Failure states.


Repository: aurora


Description
---

Added some more items to the JobUpdateAction enum. Not married to the labels 
I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
finer-grained actions. 


Expected actions:

1) instance update: INSTANCE_UPDATED
2) instance remove: INSTANCE_REMOVED
3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
4) noop: INSTANCE_SKIPPED
5) instance rolled back: INSTANCE_ROLLED_BACK


Diffs (updated)
-

  src/main/thrift/org/apache/aurora/gen/api.thrift 
af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
f695b85514bcc5cedb16e962124af3db052cb17a 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
  src/test/resources/org/apache/aurora/gen/api.thrift.md5 
21a05f6939da1dd7fc15cf6336bc3fee283f16ab 

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


Testing
---

./gradlew -Pq build


Thanks,

David McLaughlin



Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Aug. 18, 2014, 5:49 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 18, 2014, 5:49 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On Aug. 18, 2014, 5:49 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 18, 2014, 5:49 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread David McLaughlin

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

(Updated Aug. 18, 2014, 5:59 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

rebase. (also, a reminder that I need someone to commit this for me.)


Repository: aurora


Description
---

Added some more items to the JobUpdateAction enum. Not married to the labels 
I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
finer-grained actions. 


Expected actions:

1) instance update: INSTANCE_UPDATED
2) instance remove: INSTANCE_REMOVED
3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
4) noop: INSTANCE_SKIPPED
5) instance rolled back: INSTANCE_ROLLED_BACK


Diffs (updated)
-

  src/main/thrift/org/apache/aurora/gen/api.thrift 
af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
f669dbe472ea2171b0ac898b71626f0b25c0a9ec 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
78798f29bfbcf04b48953d050e498b3cad2248fb 
  src/test/resources/org/apache/aurora/gen/api.thrift.md5 
21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
  src/test/resources/org/apache/aurora/gen/storage.thrift.md5 
45762990d33969bedde7340887cde16a535e99fe 

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


Testing
---

./gradlew -Pq build


Thanks,

David McLaughlin



Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner

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

Ship it!


This is now on master, thanks!

commit 4f6093b
Author: David McLaughlin da...@dmclaughlin.com
Date:   Mon Aug 18 11:27:16 2014 -0700

Expand actions in JobUpdateAction

Reviewed at https://reviews.apache.org/r/24720/

- Bill Farner


On Aug. 18, 2014, 5:59 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 18, 2014, 5:59 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f669dbe472ea2171b0ac898b71626f0b25c0a9ec 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 78798f29bfbcf04b48953d050e498b3cad2248fb 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
   src/test/resources/org/apache/aurora/gen/storage.thrift.md5 
 45762990d33969bedde7340887cde16a535e99fe 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-15 Thread David McLaughlin

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

(Updated Aug. 15, 2014, 5:52 p.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description (updated)
---

Added some more items to the JobUpdateAction enum. Not married to the labels 
I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
finer-grained actions. 


Expected actions:

1) instance update: INSTANCE_UPDATED
2) instance remove: INSTANCE_REMOVED
3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
4) noop: INSTANCE_SKIPPED
5) instance rolled back: INSTANCE_ROLLED_BACK


Diffs (updated)
-

  src/main/thrift/org/apache/aurora/gen/api.thrift 
af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
f695b85514bcc5cedb16e962124af3db052cb17a 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
  src/test/resources/org/apache/aurora/gen/api.thrift.md5 
21a05f6939da1dd7fc15cf6336bc3fee283f16ab 

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


Testing (updated)
---

./gradlew -Pq build


Thanks,

David McLaughlin



Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-15 Thread Maxim Khutornenko

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



src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/24720/#comment88609

Rolling back assumes removing new (INSTANCE_REMOVED) and adding old 
(INSTANCE_ADDED). Do we want to distinct those actions from the forward roll?


- Maxim Khutornenko


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-15 Thread David McLaughlin


 On Aug. 15, 2014, 6:02 p.m., Maxim Khutornenko wrote:
  src/main/thrift/org/apache/aurora/gen/api.thrift, line 558
  https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line558
 
  Rolling back assumes removing new (INSTANCE_REMOVED) and adding old 
  (INSTANCE_ADDED). Do we want to distinct those actions from the forward 
  roll?

INSTANCE_REMOVED is only used when you remove an instance because the new 
instanceCount is less than previous one, it's not used if you're removing an 
instance as part of updating it.


- David


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


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-15 Thread Maxim Khutornenko

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



src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/24720/#comment88672

What action is going to be used for an instance that failed to update and 
the job rollback is disabled?


- Maxim Khutornenko


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 15, 2014, 5:52 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Expected actions:
 
 1) instance update: INSTANCE_UPDATED
 2) instance remove: INSTANCE_REMOVED
 3) instance add: INSTANCE_ADDED - INSTANCE_UPDATING - INSTANCE_UPDATED
 4) noop: INSTANCE_SKIPPED
 5) instance rolled back: INSTANCE_ROLLED_BACK
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  f695b85514bcc5cedb16e962124af3db052cb17a 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ebcb9103d75909080f5b6a69db3a1bf46cfd9780 
   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-14 Thread Bill Farner

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

Ship it!



src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/24720/#comment88542

Your call, but you might benefit from an INSTANCE_ROLLING_BACK value as 
well.


- Bill Farner


On Aug. 14, 2014, 11:21 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 14, 2014, 11:21 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-14 Thread Maxim Khutornenko

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


The ADD_INSTANCES is being used in quite a few places in unit tests. Please, 
update those and the thrift checksum.

- Maxim Khutornenko


On Aug. 14, 2014, 11:21 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 14, 2014, 11:21 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David McLaughlin
 




Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-14 Thread Bill Farner

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


Also, while it might seem tedious, can you include in the description the 
events you would expect for the success and failure (revert) cases of:

1. instance update
2. instance remove
3. instance add
4. noop

This will help prove the enum is complete, and help me follow the same policy 
when implementing the backend.

- Bill Farner


On Aug. 14, 2014, 11:21 p.m., David McLaughlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24720/
 ---
 
 (Updated Aug. 14, 2014, 11:21 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added some more items to the JobUpdateAction enum. Not married to the labels 
 I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into 
 finer-grained actions. 
 
 
 Diffs
 -
 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
 
 Diff: https://reviews.apache.org/r/24720/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David McLaughlin