> On Feb. 4, 2016, 2:39 a.m., John Sirois wrote:
> > This looks about right to me.
> > 
> > In the general case though for thrift field moves (I was thinking about 
> > this problem last night), it seems to me there would need to be a 
> > bi-directional fill in-place when the new field locations were introduced 
> > to both copy the old field location value (when non-null) to the new 
> > location for old releases and vice-versa (the case you have covered here) 
> > to allow for rollback from a release that removes the old field locations.
> > 
> > IIUC the forward-fill missing here occurred long-ago though, so we're ok.
> > 
> > If that makes sense is that about right?
> 
> John Sirois wrote:
>     ... and furthermore, the bi-drectional fill pattern is needed in-general 
> to cope with the fact that thrift objects are values equals, so any field 
> move requires bi-directional fill to be safe for sets and maps and the like.

We have switched to JobKeys in 0.6.0: 
https://github.com/apache/aurora/blob/rel/0.6.0-incubating/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L125-L128

Now that we are fully relying on TaskConfig.job, I don't think forward filling 
is needed any longer. There is simply no way to have a TaskConfig without a job 
field populated. The opposite is not true as we learned in AURORA-1603. 

In general though, I agree that every field deprecation should be followed by a 
bi-directional backfill to set up forward (introduction of new fields) and 
backward (removal of deprecated fields) deploys.


- Maxim


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


On Feb. 4, 2016, 2:14 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43172/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2016, 2:14 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Bill Farner.
> 
> 
> Bugs: AURORA-1603
>     https://issues.apache.org/jira/browse/AURORA-1603
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This change ensures we can un-revert https://reviews.apache.org/r/43104/ when 
> 0.12.0 ships and still have a graceful rollback from 0.13.0 to 0.12.0.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 27f1f33ffc782b3d2a2a9add494c04911659e217 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> db90150b48c5b134dde6c69f70ebca82bfdd0c12 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  54defc256ad8a261c6b56ee06ad7fdd16a26b057 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7382eca281eeab17d407ed140f16d6a633d8ad72 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  806f50d57e44261e3a1988a3c9bb742442badde8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43172/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to