Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Stephan Erb 


> On April 19, 2017, 6:32 p.m., Stephan Erb wrote:
> > I am not sure I understand your description. If you want faster updates, 
> > why don't you just reduce `watch_secs`? 
> > 
> > I understand `watch_secs` as the the time that we always wait _after_ we 
> > have entered `RUNNING`. So no task should be in `STARTING` state when we 
> > are watching. The documentation seems to agree with that: `Minimum number 
> > of seconds a shard must remain in ```RUNNING``` state before considered a 
> > success (Default: 45)`.
> > 
> > (I haven't looked into the code yet, but maybe you can clarify before I do).
> 
> Santhosh Kumar Shanmugham wrote:
> That was the intent initially. However we risk delaying all updates 
> without any changes to `watch_secs`. That is the main concern here. Let me 
> know if this will be a breaking for you.
> 
> Zameer Manji wrote:
> It's breaking for me. Instead can we just change the default value of 
> `watch_secs` to be 0?
> 
> Santhosh Kumar Shanmugham wrote:
> That is possible. It just means its not an opt-in change anymore.
> 
> Zameer Manji wrote:
> I'm ok to change the defaults and have our upgrades wildly fast by 
> default. Users can tweak health checks to have slower upgrades and only use 
> this parameter to have a fixed amount of time after an instance. This value 
> is the most brittle and imprecise.

Changing the default sounds good for me as well.


- Stephan


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


On April 19, 2017, 6:18 p.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 6:18 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Zameer Manji 


> On April 19, 2017, 9:32 a.m., Stephan Erb wrote:
> > I am not sure I understand your description. If you want faster updates, 
> > why don't you just reduce `watch_secs`? 
> > 
> > I understand `watch_secs` as the the time that we always wait _after_ we 
> > have entered `RUNNING`. So no task should be in `STARTING` state when we 
> > are watching. The documentation seems to agree with that: `Minimum number 
> > of seconds a shard must remain in ```RUNNING``` state before considered a 
> > success (Default: 45)`.
> > 
> > (I haven't looked into the code yet, but maybe you can clarify before I do).
> 
> Santhosh Kumar Shanmugham wrote:
> That was the intent initially. However we risk delaying all updates 
> without any changes to `watch_secs`. That is the main concern here. Let me 
> know if this will be a breaking for you.
> 
> Zameer Manji wrote:
> It's breaking for me. Instead can we just change the default value of 
> `watch_secs` to be 0?
> 
> Santhosh Kumar Shanmugham wrote:
> That is possible. It just means its not an opt-in change anymore.

I'm ok to change the defaults and have our upgrades wildly fast by default. 
Users can tweak health checks to have slower upgrades and only use this 
parameter to have a fixed amount of time after an instance. This value is the 
most brittle and imprecise.


- Zameer


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


On April 19, 2017, 9:18 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 9:18 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Santhosh Kumar Shanmugham 


> On April 19, 2017, 9:32 a.m., Stephan Erb wrote:
> > I am not sure I understand your description. If you want faster updates, 
> > why don't you just reduce `watch_secs`? 
> > 
> > I understand `watch_secs` as the the time that we always wait _after_ we 
> > have entered `RUNNING`. So no task should be in `STARTING` state when we 
> > are watching. The documentation seems to agree with that: `Minimum number 
> > of seconds a shard must remain in ```RUNNING``` state before considered a 
> > success (Default: 45)`.
> > 
> > (I haven't looked into the code yet, but maybe you can clarify before I do).
> 
> Santhosh Kumar Shanmugham wrote:
> That was the intent initially. However we risk delaying all updates 
> without any changes to `watch_secs`. That is the main concern here. Let me 
> know if this will be a breaking for you.
> 
> Zameer Manji wrote:
> It's breaking for me. Instead can we just change the default value of 
> `watch_secs` to be 0?

That is possible. It just means its not an opt-in change anymore.


- Santhosh Kumar


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


On April 19, 2017, 9:18 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 9:18 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Zameer Manji 


> On April 19, 2017, 9:32 a.m., Stephan Erb wrote:
> > I am not sure I understand your description. If you want faster updates, 
> > why don't you just reduce `watch_secs`? 
> > 
> > I understand `watch_secs` as the the time that we always wait _after_ we 
> > have entered `RUNNING`. So no task should be in `STARTING` state when we 
> > are watching. The documentation seems to agree with that: `Minimum number 
> > of seconds a shard must remain in ```RUNNING``` state before considered a 
> > success (Default: 45)`.
> > 
> > (I haven't looked into the code yet, but maybe you can clarify before I do).
> 
> Santhosh Kumar Shanmugham wrote:
> That was the intent initially. However we risk delaying all updates 
> without any changes to `watch_secs`. That is the main concern here. Let me 
> know if this will be a breaking for you.

It's breaking for me. Instead can we just change the default value of 
`watch_secs` to be 0?


- Zameer


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


On April 19, 2017, 9:18 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 9:18 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Santhosh Kumar Shanmugham 


> On April 19, 2017, 9:32 a.m., Stephan Erb wrote:
> > I am not sure I understand your description. If you want faster updates, 
> > why don't you just reduce `watch_secs`? 
> > 
> > I understand `watch_secs` as the the time that we always wait _after_ we 
> > have entered `RUNNING`. So no task should be in `STARTING` state when we 
> > are watching. The documentation seems to agree with that: `Minimum number 
> > of seconds a shard must remain in ```RUNNING``` state before considered a 
> > success (Default: 45)`.
> > 
> > (I haven't looked into the code yet, but maybe you can clarify before I do).

That was the intent initially. However we risk delaying all updates without any 
changes to `watch_secs`. That is the main concern here. Let me know if this 
will be a breaking for you.


- Santhosh Kumar


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


On April 19, 2017, 9:18 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 9:18 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Stephan Erb 


> On April 19, 2017, 6:31 p.m., Zameer Manji wrote:
> > "Without this tasks will wait for watch_secs after the task moves to 
> > RUNNING extending the update time."
> > 
> > I thought this was intentional. One can use this parameter to naturally 
> > slow down update speed. I was under the impression that `watch_secs` should 
> > be set to `0` if you want purely health check driven updates.

Race contion :). I have the same understanding as you.


- Stephan


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


On April 19, 2017, 6:18 p.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 6:18 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Aurora ReviewBot 

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



Master (b847db8) is red with this patch.
  ./build-support/jenkins/build.sh

  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$8
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$7
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$9
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$4
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$3
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$6
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$5
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$2
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$1
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/LogStorage$Settings
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/LogStorage$ScheduledExecutorSchedulingService
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/LogStorageModule
  Test coverage missing for 
org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl$SnapshotField
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/TemporaryStorage$TemporaryStorageFactory$1
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/BackupModule
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/Recovery$RecoveryImpl
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/TemporaryStorage$TemporaryStorageFactory
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/Recovery$RecoveryImpl$PendingRecovery
  Test coverage missing for org/apache/aurora/scheduler/TaskVars
  Test coverage missing for 
org/apache/aurora/scheduler/SchedulerLifecycle$DefaultDelayedActions
  Test coverage missing for 
org/apache/aurora/scheduler/TierManager$TierManagerImpl$TierConfig
  Test coverage missing for org/apache/aurora/scheduler/TaskVars$Counter
  Test coverage missing for org/apache/aurora/scheduler/TaskVars$1
  Test coverage missing for 
org/apache/aurora/scheduler/SchedulerModule$TaskEventBatchWorker
  Test coverage missing for org/apache/aurora/scheduler/HostOffer$1
  Test coverage missing for org/apache/aurora/scheduler/SchedulerModule
  Test coverage missing for 
org/apache/aurora/scheduler/TaskIdGenerator$TaskIdGeneratorImpl
  Test coverage missing for org/apache/aurora/scheduler/SchedulerModule$1
  Test coverage missing for org/apache/aurora/scheduler/TaskStatusHandlerImpl
  Test coverage missing for org/apache/aurora/scheduler/TaskStatusHandlerImpl$1
  Test coverage missing for org/apache/aurora/scheduler/TierModule
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/VolumeModeTypeHandler

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

BUILD FAILED

Total time: 12 mins 14.443 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On April 19, 2017, 4:18 p.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 4:18 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Stephan Erb 

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



I am not sure I understand your description. If you want faster updates, why 
don't you just reduce `watch_secs`? 

I understand `watch_secs` as the the time that we always wait _after_ we have 
entered `RUNNING`. So no task should be in `STARTING` state when we are 
watching. The documentation seems to agree with that: `Minimum number of 
seconds a shard must remain in ```RUNNING``` state before considered a success 
(Default: 45)`.

(I haven't looked into the code yet, but maybe you can clarify before I do).

- Stephan Erb


On April 19, 2017, 6:18 p.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 6:18 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Zameer Manji 

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



"Without this tasks will wait for watch_secs after the task moves to RUNNING 
extending the update time."

I thought this was intentional. One can use this parameter to naturally slow 
down update speed. I was under the impression that `watch_secs` should be set 
to `0` if you want purely health check driven updates.

- Zameer Manji


On April 19, 2017, 9:18 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 9:18 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Santhosh Kumar Shanmugham 

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

(Updated April 19, 2017, 9:18 a.m.)


Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.


Changes
---

Feedback.


Repository: aurora


Description
---

Tasks are gated on health-checks before they transistion into `RUNNING`
and hence it is possible for a task to stay in `STARTING` during the
watch duration of an instance's update. So include `STARTING` into the
possible states for a task when watching it after an update. Without
this tasks will wait for watch_secs after the task moves to RUNNING
extending the update time.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
c129896d8cd54abd2634e2a339c27921042b0162 
  src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
c95943d242dc2f539778bdc9e071f342005e8de3 
  src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
32a8c89d3e71395696fc613da96b871330891c42 


Diff: https://reviews.apache.org/r/58524/diff/2/

Changes: https://reviews.apache.org/r/58524/diff/1-2/


Testing
---

./build-support/jenkins/build.sh
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Santhosh Kumar Shanmugham

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread David McLaughlin 

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


Ship it!




Ship It!

- David McLaughlin


On April 19, 2017, 9:07 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 9:07 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Joshua Cohen 

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


Ship it!




lgtm


src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java
Lines 51 (patched)


0L


- Joshua Cohen


On April 19, 2017, 9:07 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 9:07 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Santhosh Kumar Shanmugham 

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



@ReviewBot retry

- Santhosh Kumar Shanmugham


On April 19, 2017, 2:07 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 2:07 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Santhosh Kumar Shanmugham 


> On April 19, 2017, 2:12 a.m., Aurora ReviewBot wrote:
> > Master (b847db8) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > :commons:classesThriftNote: Some input files use unchecked or unsafe 
> > operations.
> > Note: Recompile with -Xlint:unchecked for details.
> > 
> > :commons-args:compileJava
> > :commons-args:processResources
> > :commons-args:classes
> > :commons-args:jar
> > :commons:compileJavaNote: Writing 
> > file:/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.1
> > Note: Writing 
> > file:/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
> > Note: Some input files use or override a deprecated API.
> > Note: Recompile with -Xlint:deprecation for details.
> > Note: Some input files use unchecked or unsafe operations.
> > Note: Recompile with -Xlint:unchecked for details.
> > 
> > :commons:generateThriftResources
> > :commons:processResources
> > :commons:classes
> > :commons:jar
> > :compileJava
> > Download 
> > https://repo1.maven.org/maven2/org/apache/mesos/mesos/1.2.0/mesos-1.2.0.pom
> > Download 
> > https://repo1.maven.org/maven2/org/apache/mesos/mesos/1.2.0/mesos-1.2.0.jar
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74:
> >  Note: Wrote forwarder 
> > org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
> > @Forward({
> > ^
> > Note: Writing 
> > file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
> > Note: Writing 
> > file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
> > :generateBuildProperties
> > :processResources
> > :classes
> > :jar
> > :startScripts
> > :distTar
> > :distZip
> > :assemble
> > :compileJmhJavaJava HotSpot(TM) 64-Bit Server VM warning: INFO: 
> > os::commit_memory(0x00079fc0, 243269632, 0) failed; error='Cannot 
> > allocate memory' (errno=12)
> > #
> > # There is insufficient memory for the Java Runtime Environment to continue.
> > # Native memory allocation (mmap) failed to map 243269632 bytes for 
> > committing reserved memory.
> > # An error report file with more information is saved as:
> > # /home/jenkins/jenkins-slave/workspace/AuroraBot/hs_err_pid11304.log
> > 
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"

@ReviewBot retry


- Santhosh Kumar


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


On April 19, 2017, 2:07 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 2:07 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Aurora ReviewBot 

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



Master (b847db8) is red with this patch.
  ./build-support/jenkins/build.sh

:commons:classesThriftNote: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:commons-args:compileJava
:commons-args:processResources
:commons-args:classes
:commons-args:jar
:commons:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.1
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:commons:generateThriftResources
:commons:processResources
:commons:classes
:commons:jar
:compileJava
Download 
https://repo1.maven.org/maven2/org/apache/mesos/mesos/1.2.0/mesos-1.2.0.pom
Download 
https://repo1.maven.org/maven2/org/apache/mesos/mesos/1.2.0/mesos-1.2.0.jar
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
:generateBuildProperties
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaJava HotSpot(TM) 64-Bit Server VM warning: INFO: 
os::commit_memory(0x00079fc0, 243269632, 0) failed; error='Cannot 
allocate memory' (errno=12)
#
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (mmap) failed to map 243269632 bytes for committing 
reserved memory.
# An error report file with more information is saved as:
# /home/jenkins/jenkins-slave/workspace/AuroraBot/hs_err_pid11304.log


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On April 19, 2017, 9:07 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58524/
> ---
> 
> (Updated April 19, 2017, 9:07 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Tasks are gated on health-checks before they transistion into `RUNNING`
> and hence it is possible for a task to stay in `STARTING` during the
> watch duration of an instance's update. So include `STARTING` into the
> possible states for a task when watching it after an update. Without
> this tasks will wait for watch_secs after the task moves to RUNNING
> extending the update time.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> c129896d8cd54abd2634e2a339c27921042b0162 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> c95943d242dc2f539778bdc9e071f342005e8de3 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 32a8c89d3e71395696fc613da96b871330891c42 
> 
> 
> Diff: https://reviews.apache.org/r/58524/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Santhosh Kumar Shanmugham 

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

Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.


Repository: aurora


Description
---

Tasks are gated on health-checks before they transistion into `RUNNING`
and hence it is possible for a task to stay in `STARTING` during the
watch duration of an instance's update. So include `STARTING` into the
possible states for a task when watching it after an update. Without
this tasks will wait for watch_secs after the task moves to RUNNING
extending the update time.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
c129896d8cd54abd2634e2a339c27921042b0162 
  src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
c95943d242dc2f539778bdc9e071f342005e8de3 
  src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
df1f8394b824dbb7b2745fcccdab5adaafdf6e6c 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
32a8c89d3e71395696fc613da96b871330891c42 


Diff: https://reviews.apache.org/r/58524/diff/1/


Testing
---

./build-support/jenkins/build.sh
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Santhosh Kumar Shanmugham