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

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,

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,

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.

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

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,

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 ---

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.

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,

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,

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

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 ---

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

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.

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,

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!

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,

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.

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 ---

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,

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

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.

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,

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(

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.

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,

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 > >

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 > >

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 > >

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.

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 > >

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,

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 > >

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

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

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.

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 > >

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"

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,

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

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

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

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/#review143102 --- Don't forget to highlight your work in the changelog!

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 ---

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.

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,

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,

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,

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.

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

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.

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

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/ --- Review request for Aurora. Repository: aurora Description --- Add