> On Feb. 4, 2016, 6:21 a.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.
> 
> Maxim Khutornenko wrote:
>     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?

Perhaps just a cautionary comment here is sufficient for now.  Given that we 
are assuming an invariant with `Maps.uniqueIndex`, an object inserted into the 
DB must be `equals()` the same object when retrieved.


- Bill


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


On Feb. 3, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43172/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2016, 6:14 p.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