----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58036/#review170456 -----------------------------------------------------------
This may be an existing problem with the current impl as well, but what happens if we drop an enum value? Are we just assuming some other migration will have removed it (since presumably other tables will need to be updated to have their FK relations fixed)? - Joshua Cohen On March 29, 2017, 7:21 p.m., Zameer Manji wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58036/ > ----------------------------------------------------------- > > (Updated March 29, 2017, 7:21 p.m.) > > > Review request for Aurora, Santhosh Kumar Shanmugham and Stephan Erb. > > > Bugs: AURORA-1912 > https://issues.apache.org/jira/browse/AURORA-1912 > > > Repository: aurora > > > Description > ------- > > In our in memory database, we model enums as two column tables. The two > columns > would be `id` which corresponds to the integer value in the thrift enum and > `name` which is the all caps string name of the enum. For example to model the > `JobUpdateStatus` enum we have a table called `job_update_statuses`. In there > the `ROLLING_FORWARD` enum is modeled as a row `(0, "ROLLING_FORWARD")`. Other > tables reference the enum table via the id. > > When we prepare storage on startup the `DbStorage` starts up. It does two > things: > 1. Load in the schema. > 2. Populate the enum tables. > > This ensures that when we insert values into the database, the enum refernces > will be valid. > > However, after we restore from a Snapshot with the `dbScript` field, we blow > all > of that data away and restore what was in the snapshot: > ```` > try (Connection c = ((DataSource) > store.getUnsafeStoreAccess()).getConnection()) { > LOG.info("Dropping all tables"); > try (PreparedStatement drop = c.prepareStatement("DROP ALL OBJECTS")) \ > drop.executeUpdate(); > } > ```` > > This means that if we add a new enum value, and then restore from a snapshot, > that enum value will not exist in the table any more. We could address this by > saying that every enum value addition requires a migration. However instead I > propose not blowing away the work done by `DbStorage` instead and re-hydrating > the enum tables. > > To do this I extracted the logic into a new class `EnumBackfill`. Restoring > from > a snapshot calls this after the migrations are done. The underlying SQL was > changed from `INSERT` to `MERGE` to make this work. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > f0b148cd158d61cd89cc51dca9f3fa4c6feb1b49 > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > 36a1bd5c784ed0febebccfd22e5064f0b2e3106f > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java > d1a196419b67108ee2bb778f83a2993e2e5ee83b > src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java > 923e904f396724b9dde4a330ef312a6aae2c02a6 > src/main/java/org/apache/aurora/scheduler/storage/db/EnumBackfill.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java > 81a8cca6974e33c774473a4990e0e981cf6ddee6 > > src/main/resources/org/apache/aurora/scheduler/storage/db/EnumValueMapper.xml > 153fd26c27275c46b190e71d8a5736153f2c2d18 > src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java > 42615da54096d5b06c7989cb30fc3cfbe59bc1b9 > src/test/java/org/apache/aurora/scheduler/storage/db/DbStorageTest.java > f26529c76214c8f22563f04a197798c82d341b49 > > src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java > ca9525665805a33b4a322a72022ff037f0dd2a94 > > > Diff: https://reviews.apache.org/r/58036/diff/1/ > > > Testing > ------- > > existing tests and e2e tests > > I also added a new enum value to `JobUpdateStatus` and observed it was > correctly loaded in. > > > Thanks, > > Zameer Manji > >
