----------------------------------------------------------- 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) <https://reviews.apache.org/r/50168/#comment208868> this should be `2: string message`. This is a new RPC so we should be naming all of the arguments from 1. src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java (line 136) <https://reviews.apache.org/r/50168/#comment208870> +1 to accepting just the JobUpdateKey instead of the whole update. src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java (lines 129 - 131) <https://reviews.apache.org/r/50168/#comment208873> +1 I'm fine with having this "undo" any of the existing updates. I can imagine scenarios where a user of a cluster just wants to revert to a "last known good state" (ie they deploy bad code or configuration twice). src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java (lines 279 - 280) <https://reviews.apache.org/r/50168/#comment208875> +1 I think this can be removed, it seems to be a left over of a previous approach. src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java (line 282) <https://reviews.apache.org/r/50168/#comment208876> +1 It will match the "Attempting to rollback update " log line at the start. src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java (line 1367) <https://reviews.apache.org/r/50168/#comment208874> We need tests to ensure that we can only rollback updates in the `ROLLING_FORWARD` and the `ROLLED_FORWARD` state when there is no other update in progress. - Zameer Manji On July 20, 2016, 8:37 p.m., Igor Morozov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50168/ > ----------------------------------------------------------- > > (Updated July 20, 2016, 8:37 p.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 > >
