> On July 21, 2016, 4:05 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > lines 279-280 > > <https://reviews.apache.org/r/50168/diff/3/?file=1448819#file1448819line279> > > > > What is the purpose of this call if it's not using the return value? As > > far as I can tell there are no side effects from the call.
This is additional verification step similar to start() function. I think this step is not nessesary if updateFactory.newUpdate(update.getInstructions(), true) == updateFactory.newUpdate(update.getInstructions(), false). It was not clear from a code that this is the case. https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java#L162 > On July 21, 2016, 4:05 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, > > lines 129-131 > > <https://reviews.apache.org/r/50168/diff/3/?file=1448818#file1448818line129> > > > > What if the update is in `ROLLED_FORWARD` and another update has > > already run and completed since this one (i.e. there's no update in > > `ROLLING_FORWARD`, but there is a more recent update in `ROLLED_FORWARD`)? we should be able to rollback any update to its initial state. I'll correct the comment. > On July 21, 2016, 4:05 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > line 251 > > <https://reviews.apache.org/r/50168/diff/3/?file=1448819#file1448819line251> > > > > I might be missing something, but I don't see a check in this method > > that if the update is complete, it's in the `ROLLED_FORWARD` state (as > > described by the javadoc on the interface declaration). I am relying here on JobUpdate state machine that whould prevent unallowed state transitions - Igor ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50168/#review143084 ----------------------------------------------------------- On July 21, 2016, 3:37 a.m., Igor Morozov wrote: > > ----------------------------------------------------------- > 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, Maxim Khutornenko, Bill Farner, and > Zameer Manji. > > > Bugs: AURORA-1721 > https://issues.apache.org/jira/browse/AURORA-1721 > > > Repository: aurora > > > Description > ------- > > Add rollback functionality to the scheduler > > Thic change also includes an addition to storage replicated log. I needed a > way to re-insert lock token for an existing job update. > > > Diffs > ----- > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > 1d66208490aff6ea8af4c737845fa2cf13617529 > api/src/main/thrift/org/apache/aurora/gen/storage.thrift > 9e4213f13255a182df938bea44ca87fa03a25318 > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java > 52c3c6618a3cf1009435ca8a9cece36365913e55 > src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java > d2673e6b328cb1e249fbe91d18e0d9e935636eaa > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > 39924c62108f6a68f545f90d77ceab4265d41fad > > src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java > d0de063fd78e6c4f62fae4a598d1d22f9775772d > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > b534abf95bab6e1657e3ef993cf34c0d6ec460be > > src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java > 9243c92b11040b68ed6014b3991db69fc08bcddf > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java > f8357c46df1b025bf4e38a7ce1cb1c13a50c39f9 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > 364c5c753f884a2d89e27802d7bbf3b2b6d3a08e > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/update.py > bb526f7bf94d7bfe02fe2786493c85be1bfeb86f > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > e157c0dfde5efc418448e138aa008ade742fe816 > src/test/python/apache/aurora/client/api/test_scheduler_client.py > afbd385b7eda64cb1f7d118b695e65e4045eac6c > > Diff: https://reviews.apache.org/r/50168/diff/ > > > Testing > ------- > > > Thanks, > > Igor Morozov > >
