Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review145558 --- Ship it! LGTM, it seems maxim and josh have really dug into the

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 11, 2016, 6:07 p.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review145455 --- Ship it! Ship It! - Maxim Khutornenko On Aug. 11, 2016, 1:32

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review145453 --- Ship it! Master (9084664) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

> On Aug. 10, 2016, 5:14 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, > > lines 131-133 > > > > > > This needs to be updated to include ROLL_FORWARD

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 11, 2016, 1:32 a.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review145356 --- src/main/java/org/apache/aurora/scheduler/updater/JobUpdateContro

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review145302 --- Ship it! Thanks for adding that transition. This looks good to

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review145289 --- Ship it! Master (581262c) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 9, 2016, 10:41 p.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 9, 2016, 10:31 p.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review145283 --- This patch does not apply cleanly against master (581262c), do you

Re: Review Request 50168: Add rollback functionality to the scheduler

> On Aug. 8, 2016, 10:54 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, > > line 133 > > > > > > It feels like we should also be able to roll back an updat

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review145144 --- src/main/java/org/apache/aurora/scheduler/updater/JobUpdateContro

Re: Review Request 50168: Add rollback functionality to the scheduler

> On Aug. 3, 2016, 10:48 p.m., Aurora ReviewBot wrote: > > Master (a071af3) is green with this patch. > > ./build-support/jenkins/build.sh > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" jcohen, zmanji, wfarner any additional feedback on this r

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144689 --- Ship it! Master (a071af3) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 3, 2016, 10:16 p.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144678 --- Ship it! Thanks for following up! src/main/java/org/apache/au

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 3, 2016, 9:51 p.m.) Review request for Aurora, Joshua Cohen, Maxi

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144674 --- Ship it! Master (a071af3) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144669 --- src/main/java/org/apache/aurora/scheduler/updater/JobUpdateContro

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 3, 2016, 8:47 p.m.) Review request for Aurora, Joshua Cohen, Maxi

Re: Review Request 50168: Add rollback functionality to the scheduler

> On Aug. 2, 2016, 9:53 p.m., Stephan Erb wrote: > > RELEASE-NOTES.md, line 38 > > > > > > That slipped into the 0.15er release notes rather than 0.16. +1. Also, please mention this feature is only applicable to acti

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144552 --- LGTM overall. Only some polishing left. CHANGELOG (line 4)

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144539 --- RELEASE-NOTES.md (line 38)

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144533 --- Ship it! Master (b912e17) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 2, 2016, 9:20 p.m.) Review request for Aurora, Joshua Cohen, Maxi

Re: Review Request 50168: Add rollback functionality to the scheduler

Pretty much. The above has to be called from the storage.write transaction. That and the active update checks would go into SchedulerThriftInterface RPC implementation. On Tue, Aug 2, 2016 at 12:04 PM, Igor Morozov wrote: > you mean just do this: > unscopedChangeUpdateStatus( > key,

Re: Review Request 50168: Add rollback functionality to the scheduler

> On Aug. 2, 2016, 1:55 a.m., Aurora ReviewBot wrote: > > Master (b912e17) is green with this patch. > > ./build-support/jenkins/build.sh > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" Inducing a rollback for an active update can be done much

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144438 --- Ship it! Master (b912e17) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated Aug. 2, 2016, 1:26 a.m.) Review request for Aurora, Joshua Cohen, Maxi

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 29, 2016, 7:25 p.m., Maxim Khutornenko wrote: > > Finally had time to go over this review in detail. I am afraid the proposed > > approach of rolling back the arbitrary update may not work in general case > > and result in unexpected outcome, inconsistent job state and hard to > > tr

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 29, 2016, 7:25 p.m., Maxim Khutornenko wrote: > > Finally had time to go over this review in detail. I am afraid the proposed > > approach of rolling back the arbitrary update may not work in general case > > and result in unexpected outcome, inconsistent job state and hard to > > tr

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 29, 2016, 7:25 p.m., Maxim Khutornenko wrote: > > Finally had time to go over this review in detail. I am afraid the proposed > > approach of rolling back the arbitrary update may not work in general case > > and result in unexpected outcome, inconsistent job state and hard to > > tr

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144137 --- Finally had time to go over this review in detail. I am afraid the

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 28, 2016, 8:44 p.m., David McLaughlin wrote: > > For me ROLLED_BACK has a clear meaning - a job that was rolled back due to > > a failed health check in the update process. Overloading like this is going > > to lead to ambiguity and I wouldn't be comfortable exposing this > > functio

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144039 --- Ship it! Master (dde2c92) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 28, 2016, 8:44 p.m., David McLaughlin wrote: > > For me ROLLED_BACK has a clear meaning - a job that was rolled back due to > > a failed health check in the update process. Overloading like this is going > > to lead to ambiguity and I wouldn't be comfortable exposing this > > functio

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated July 28, 2016, 10:54 p.m.) Review request for Aurora, Joshua Cohen, Ma

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 28, 2016, 8:44 p.m., David McLaughlin wrote: > > For me ROLLED_BACK has a clear meaning - a job that was rolled back due to > > a failed health check in the update process. Overloading like this is going > > to lead to ambiguity and I wouldn't be comfortable exposing this > > functio

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144000 --- David makes a good comment. Maxim/Bill: What do you guys think abo

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review144001 --- For me ROLLED_BACK has a clear meaning - a job that was rolled bac

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review143981 --- Ship it! Master (dde2c92) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 25, 2016, 7:57 p.m., Maxim Khutornenko wrote: > > api/src/main/thrift/org/apache/aurora/gen/storage.thrift, lines 95-99 > > > > > > The idea of update locks is entirely obsolete now that we don't have > > c

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 21, 2016, 9:12 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, > > lines 129-131 > > > > > > +1 > > > > I'm fine with having this "undo" a

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated July 28, 2016, 7:06 p.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 21, 2016, 4:05 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > lines 279-280 > > > > > > What is the purpose of this call if it's not using

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 21, 2016, 4:05 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > lines 279-280 > > > > > > What is the purpose of this call if it's not using

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review143421 --- api/src/main/thrift/org/apache/aurora/gen/storage.thrift (lines 9

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 21, 2016, 10:05 a.m., Bill Farner wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line > > > > > > Echoing what David brought up on the mailing list - i find this RPC > > ambiguous.

Re: Review Request 50168: Add rollback functionality to the scheduler

> On July 21, 2016, 10:05 a.m., Bill Farner wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line > > > > > > Echoing what David brought up on the mailing list - i find this RPC > > ambiguous.

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review143139 --- api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 1115)

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review143102 --- Don't forget to highlight your work in the changelog! api/src/ma

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review143084 --- src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateS

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review143052 --- Ship it! Master (e67c6a7) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated July 21, 2016, 3:37 a.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated July 21, 2016, 3:35 a.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated July 19, 2016, 9:38 p.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated July 19, 2016, 9:19 p.m.) Review request for Aurora, Joshua Cohen, Max

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review142678 --- Ship it! Master (e67c6a7) is green with this patch. ./build-s

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated July 19, 2016, 12:56 a.m.) Review request for Aurora, Maxim Khutornenk

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review142670 --- Master (e67c6a7) is red with this patch. ./build-support/jenkins

Re: Review Request 50168: Add rollback functionality to the scheduler

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/ --- (Updated July 18, 2016, 11:32 p.m.) Review request for Aurora, Maxim Khutornenk