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 16:42:21 GMT


> On Feb. 4, 2016, 2:21 p.m., John Sirois wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java, line
45
> > <https://reviews.apache.org/r/43172/diff/1/?file=1232006#file1232006line45>
> >
> >     Having a backfill in the db layer strikes me as a red flag; ie all thrift inputs
to the scheduler should be sanitized as close as possible to the input site. I think this
means asap in a thrift RPC and when reading from the distributed log.
> >     
> >     This may be impractical to implement today or else this just may be more expedient
to getting 0.12.0 out the door with confidence in which case I'm fine with it, but I'd like
to file a follow-up issue to investigate a generic intercepting layer for the thrift RPC interface
and the LogStorage that could look up thrift structs types in a map and apply a sanitization
UnaryOperator if found... mod some handwaving.
> 
> Bill Farner wrote:
>     I agree with the above.  I also want to point out (and suggest a comment) that it
is imperative that the backfill routine is logically equivalent to the way a `TaskConfig`
is altered by a storage round-trip.  If at any point that does not hold true, we can encounter
the same symptom (duplicate task configs) again.

This particular backfill was, in fact, redundant given that we also do it at the recovery
level. I am going to remove it entirely.

Bill, given the above, how should I interpret your comment now? Are you suggesting remodeling
the schema to what we discussed earlier to explicitly map deprecated fields to DB columns?


- Maxim


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


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