aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zameer Manji <zma...@apache.org>
Subject Re: Review Request 58036: Ensure enum tables are complete afer a snapshot restore.
Date Wed, 29 Mar 2017 19:52:05 GMT


> On March 29, 2017, 12:34 p.m., Joshua Cohen wrote:
> > 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)?
> 
> Zameer Manji wrote:
>     Yes, exactly. Dropping an enum value requires removing other tables. I presume we
will just never use enums instead of actually deleting them.
> 
> Joshua Cohen wrote:
>     Might want to document that somewhere (even if just a comment on the backfill that
it's only for additive changes). Otherwise the general approach looks good to me.

Done.


- Zameer


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58036/#review170456
-----------------------------------------------------------


On March 29, 2017, 12:51 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, 12:51 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/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/2/
> 
> 
> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message