aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maxim Khutornenko <ma...@apache.org>
Subject Re: Review Request 43172: Add deprecated field storage backfill
Date Thu, 04 Feb 2016 04:57:23 GMT


> 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
> 
>


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