Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread David McLaughlin

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


Does removing an instance take any significant amount of time? Can it fail? The 
concern is we'll be showing 100% progress but the update is still running when 
reducing the instance count.

- David McLaughlin


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25741/
 ---
 
 (Updated Sept. 17, 2014, 5:15 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
 `JobUpdateInstructions`) to represent the delta incurred by a job update 
 (AURORA-718), i believe we can remove these two states, as addition/removal 
 can be inferred from the delta.
 
 Furthermore, this allows the job updater's internal state machine to 
 translate directly into the remaining `JobUpdateActions`, and hides details 
 of the non-atomic actions necessary to move an instance from state A to state 
 B.
 
 
 Diffs
 -
 
   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
 5d6299f9de6eccd0f1332e11d57dfb910d956011 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  e09caa63bc0150d7109cb237e80b9efee441dded 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
 
 Diff: https://reviews.apache.org/r/25741/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread Bill Farner


 On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote:
  Does removing an instance take any significant amount of time? Can it fail? 
  The concern is we'll be showing 100% progress but the update is still 
  running when reducing the instance count.

Depends on your definition of significant.  It does take a non-deterministic 
amount of time, since we transition to KILLING and await to see that the task 
is gone.  It can _sort of_ fail, but not in a way that we will record an 
instance failure AFAICT.  However, i think the behavior of ACTING - FINISHED 
even in removal is nice as it matches two-step nature of the operation.


- Bill


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


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25741/
 ---
 
 (Updated Sept. 17, 2014, 5:15 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
 `JobUpdateInstructions`) to represent the delta incurred by a job update 
 (AURORA-718), i believe we can remove these two states, as addition/removal 
 can be inferred from the delta.
 
 Furthermore, this allows the job updater's internal state machine to 
 translate directly into the remaining `JobUpdateActions`, and hides details 
 of the non-atomic actions necessary to move an instance from state A to state 
 B.
 
 
 Diffs
 -
 
   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
 5d6299f9de6eccd0f1332e11d57dfb910d956011 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  e09caa63bc0150d7109cb237e80b9efee441dded 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
 
 Diff: https://reviews.apache.org/r/25741/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread David McLaughlin


 On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote:
  Does removing an instance take any significant amount of time? Can it fail? 
  The concern is we'll be showing 100% progress but the update is still 
  running when reducing the instance count.
 
 Bill Farner wrote:
 Depends on your definition of significant.  It does take a 
 non-deterministic amount of time, since we transition to KILLING and await to 
 see that the task is gone.  It can _sort of_ fail, but not in a way that we 
 will record an instance failure AFAICT.  However, i think the behavior of 
 ACTING - FINISHED even in removal is nice as it matches two-step nature of 
 the operation.

I see, so the new behaviour is that in both scenarios the instance would end up 
in INSTANCE_UPDATED state? And then the client infers whether it was added or 
removed based on the instance count delta?


- David


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


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25741/
 ---
 
 (Updated Sept. 17, 2014, 5:15 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
 `JobUpdateInstructions`) to represent the delta incurred by a job update 
 (AURORA-718), i believe we can remove these two states, as addition/removal 
 can be inferred from the delta.
 
 Furthermore, this allows the job updater's internal state machine to 
 translate directly into the remaining `JobUpdateActions`, and hides details 
 of the non-atomic actions necessary to move an instance from state A to state 
 B.
 
 
 Diffs
 -
 
   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
 5d6299f9de6eccd0f1332e11d57dfb910d956011 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  e09caa63bc0150d7109cb237e80b9efee441dded 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
 
 Diff: https://reviews.apache.org/r/25741/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread Bill Farner


 On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote:
  Does removing an instance take any significant amount of time? Can it fail? 
  The concern is we'll be showing 100% progress but the update is still 
  running when reducing the instance count.
 
 Bill Farner wrote:
 Depends on your definition of significant.  It does take a 
 non-deterministic amount of time, since we transition to KILLING and await to 
 see that the task is gone.  It can _sort of_ fail, but not in a way that we 
 will record an instance failure AFAICT.  However, i think the behavior of 
 ACTING - FINISHED even in removal is nice as it matches two-step nature of 
 the operation.
 
 David McLaughlin wrote:
 I see, so the new behaviour is that in both scenarios the instance would 
 end up in INSTANCE_UPDATED state? And then the client infers whether it was 
 added or removed based on the instance count delta?

Effectively, yes.  The states communicate acting and finished along with 
direction (updating or rollback), and the caller has the information to 
determine what the outcome looks like (config change, added, removed).


- Bill


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


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25741/
 ---
 
 (Updated Sept. 17, 2014, 5:15 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
 `JobUpdateInstructions`) to represent the delta incurred by a job update 
 (AURORA-718), i believe we can remove these two states, as addition/removal 
 can be inferred from the delta.
 
 Furthermore, this allows the job updater's internal state machine to 
 translate directly into the remaining `JobUpdateActions`, and hides details 
 of the non-atomic actions necessary to move an instance from state A to state 
 B.
 
 
 Diffs
 -
 
   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
 5d6299f9de6eccd0f1332e11d57dfb910d956011 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  e09caa63bc0150d7109cb237e80b9efee441dded 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
 
 Diff: https://reviews.apache.org/r/25741/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread David McLaughlin

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

Ship it!


Ship It!

- David McLaughlin


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25741/
 ---
 
 (Updated Sept. 17, 2014, 5:15 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
 `JobUpdateInstructions`) to represent the delta incurred by a job update 
 (AURORA-718), i believe we can remove these two states, as addition/removal 
 can be inferred from the delta.
 
 Furthermore, this allows the job updater's internal state machine to 
 translate directly into the remaining `JobUpdateActions`, and hides details 
 of the non-atomic actions necessary to move an instance from state A to state 
 B.
 
 
 Diffs
 -
 
   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
 5d6299f9de6eccd0f1332e11d57dfb910d956011 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  e09caa63bc0150d7109cb237e80b9efee441dded 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
 
 Diff: https://reviews.apache.org/r/25741/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25741/
 ---
 
 (Updated Sept. 17, 2014, 5:15 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
 `JobUpdateInstructions`) to represent the delta incurred by a job update 
 (AURORA-718), i believe we can remove these two states, as addition/removal 
 can be inferred from the delta.
 
 Furthermore, this allows the job updater's internal state machine to 
 translate directly into the remaining `JobUpdateActions`, and hides details 
 of the non-atomic actions necessary to move an instance from state A to state 
 B.
 
 
 Diffs
 -
 
   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
 5d6299f9de6eccd0f1332e11d57dfb910d956011 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
   
 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
  e09caa63bc0150d7109cb237e80b9efee441dded 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
 
 Diff: https://reviews.apache.org/r/25741/diff/
 
 
 Testing
 ---
 
 ./gradlew build -Pq
 
 
 Thanks,
 
 Bill Farner