> On May 20, 2015, 3:17 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java, > > lines 72-76 > > <https://reviews.apache.org/r/34501/diff/1/?file=965743#file965743line72> > > > > How is the db storage going to graduate to production if it's not > > actually used when the beta flag is present? > > > > My preference is that we use whichever task store system is selected by > > the command-line in all codepaths - if backup recovery fails here the > > cluster admin can relaunch the scheduler with the H2 database disabled. > > Kevin Sweeney wrote: > A better solution here IMO is to inject a TemporaryStorageFactory rather > than call the static `DbUtil.createStorage()` factory method directly. This > allows us to configure the choice of task store implementation in one place, > rather than spread out across multiple packages, which appears to have > contributed to this issue. > > Maxim Khutornenko wrote: > We have to provide a solid/guaranteed way to recover from failure. > Chances are very high we are in recover because of the TaskStore messing up > our data. Relying on a DB task store is not a solution we can rely on in such > cases. In fact, the reason I filed this bug was exactly that - I was not able > to load an external backup due to DB task store violating some schema > constraints. > > > How is the db storage going to graduate to production if it's not > actually used when the beta flag is present? > > The graduation assumes removing the in-memory task store, right? At that > moment we will switch to a DB-based binding, which will become a default > store.
Why is putting it behind a flag not sufficient? With your proposal we can't actually test that DbStorage works for backup recovery until we make a release that supports only DbStorage. If recovery fails with DbStorage because you have the `-enable_beta_storage` flag turned up, then it seems completely reasonable to turn that flag down and try again. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34501/#review84601 ----------------------------------------------------------- On May 20, 2015, 2:47 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34501/ > ----------------------------------------------------------- > > (Updated May 20, 2015, 2:47 p.m.) > > > Review request for Aurora and Kevin Sweeney. > > > Bugs: AURORA-1322 > https://issues.apache.org/jira/browse/AURORA-1322 > > > Repository: aurora > > > Description > ------- > > Defaulting TemporaryStorage to in-memory task store. > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > 23c0c1e73a183be748199610ddf03e5d654fef74 > src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java > fe8e3f8f164732769fa0ae50a62e89c8aa77e9a5 > > src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java > 177d720b59ba601d59aada9650aba799babb9a73 > > Diff: https://reviews.apache.org/r/34501/diff/ > > > Testing > ------- > > ./gradlew -Pq build > Manual restore from backup in Vagrant. > > > Thanks, > > Maxim Khutornenko > >
