> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote: > > The mechanics of the change lgtm, but is there some other way to add schema > > / data updates to a new aurora release such that they get applied after > > recovering the log but before doing anything else? I guess its valid to > > say add it back when we need it, but I want to make sure this is the stance > > you intend; ie remove the burden from the existing codebase and push it to > > the next contributor that needs to break the schema. > > Bill Farner wrote: > That is essentially the stance i intend. My hope is that when we _do_ > need it again, the resulting approach has less code ripple than we see here. > I could be deluded, however. > > John Sirois wrote: > You may be, I'm not sure. Happy to accept that intentioned stance though. > > Maxim Khutornenko wrote: > We have killed and resurrected StorageBackfill a few times already. I > personally remember reviving it once. Not really sure what killing again and > again really gets us. Can we finally accept it's existence and keep it around > as a placeholder? > > When we _do_ need it next time, what's the storage support story for it? > Call mutateTask() for every task processed?
How about we keep the plumbing for the initialization operation, but remove StorageBackfill? That will make it easy to wire in the next backfill function, but doesn't leave us with a dead class. > When we do need it next time, what's the storage support story for it? Call > mutateTask() for every task processed? Call `mutateTask()` for every task _changed_. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42646/#review115888 ----------------------------------------------------------- On Jan. 22, 2016, 12:11 a.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42646/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2016, 12:11 a.m.) > > > Review request for Aurora, John Sirois and Maxim Khutornenko. > > > Repository: aurora > > > Description > ------- > > Related to a comment i made in https://reviews.apache.org/r/42628/, > StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`. As > you can see from the patch, there's a lot that crumbles with it. > > Since it doesn't show in the patch, StorageBackfill was only filling the > `TaskConfig.job` field in tasks and cron jobs: > https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72 > > This backfill has been in place since `0.6.0-incubating` and should be safe > to remove. > https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124 > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java > 0d3874e9632952c54ef5ae9aba78638e47c87056 > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > 376f48545eb860aad5afa028d3b96d04acc08377 > > src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java > de4ada431634fb171fab109f1923da810b361205 > src/main/java/org/apache/aurora/scheduler/storage/Storage.java > 578bb37de8853c4228e76b31f601430b7170946a > src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java > 609a6242e8918bf4a044e6e5e2c8dfe4b900192e > src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java > 4e4f8d2c0f237c4480abe101835176f7d69958db > src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java > 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 > > src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java > 728353116c8892c015a6443d8db09ba241b32230 > src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java > 8fd024a460cd948dab703b99788b1ed2d6c43bc3 > src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java > bab45670d66dfed23dd6e0339687166c9be44ba4 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java > 9fb8aad5d1c0412efc6d1176e543321ebe503e03 > > src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java > 0768ec37bbc6c3c101aa04a953a36a4af7b25963 > > src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java > 1ac41d18fd9d603c4c342f9635e116bfd055f858 > src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java > 2570464c85630a55d3e6c8653fc0307d54504e15 > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > 7382eca281eeab17d407ed140f16d6a633d8ad72 > > src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java > 3c4e2bd5eefc141a5d2c5d0e56881899816652f8 > > Diff: https://reviews.apache.org/r/42646/diff/ > > > Testing > ------- > > > Thanks, > > Bill Farner > >
