> On Dec. 9, 2017, 12:13 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/storage/durability/DurableStorage.java > > Line 294 (original), 300 (patched) > > <https://reviews.apache.org/r/64459/diff/1/?file=1911719#file1911719line300> > > > > Did the recovery with a `Lock` op fail here? If so, i'm tempted to > > just change this to log and avoid the extra deprecation step.
Yep! I think it would be safer to go with the extra deprecation step and not log unknown Ops. Despite being a bit of a pain, it forces us to think about forwards and backwards compatibility when adding/removing Ops. Failing on recovery is much more preferable than recovering with unknown issues and then persisting bad state. - Jordan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64459/#review193287 ----------------------------------------------------------- On Dec. 8, 2017, 8:08 p.m., Jordan Ly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64459/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2017, 8:08 p.m.) > > > Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner. > > > Bugs: AURORA-1959 > https://issues.apache.org/jira/browse/AURORA-1959 > > > Repository: aurora > > > Description > ------- > > Deprecated Ops re-added, perform no-op instead of throwing an exception. > > BEFORE MERGING: what is the rollback story for updates? If we upgrade to 0.20 > and then revert to 0.19, there will be no locks for in-progress updates. Will > this be an issue or was saving locks essentially a no-op before? > > > Diffs > ----- > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > c9f42108e85af563ef584f3b60c0ec2ceb3f0bb6 > api/src/main/thrift/org/apache/aurora/gen/storage.thrift > 22104979ce8844929f439c44b0f4c63bf90f07d7 > > src/main/java/org/apache/aurora/scheduler/storage/durability/DurableStorage.java > 85b2113631586f43d854c4d2812f43b7b864d452 > > src/test/java/org/apache/aurora/scheduler/storage/durability/DurableStorageTest.java > 07912b6a8ffb4bd3f87861f8c17242f6056aaf49 > > > Diff: https://reviews.apache.org/r/64459/diff/1/ > > > Testing > ------- > > `./gradlew test` > > I tested by reproducing the issue (adding a job update) and then replaying > the log. Before the patch, the scheduler crashed on `recover`. This patch > succeeded. > > > Thanks, > > Jordan Ly > >
