You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2016/04/21 19:08:22 UTC

Review Request 46499: Revert "Moving db migration into LogStorage"

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

Review request for Aurora and Joshua Cohen.


Repository: aurora


Description
-------

This reverts commit ae051f3b92797d5c9f328c6c6d42d03ee4077938.

Moving the migrator call back to snapshot store.

I realized we can't rely on post-recover data migration alone to satisfy backfilling needs. During recovery, we perform various insertion calls (mem store or log storage catch up) that rely on thrift schema matching mybatis schema. This may not always be true (as we learned in AURORA-1603) and may result in incorrect or duplicate data being inserted.

The correct data migration sequence for all configuration cases should be:
1. migrate schema
2. migrate any data loaded from dbsnapshot (if applicable)
3. apply snapshot with backfilling
4. replay log store transactions with backfilling


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 5c7d92f00ddda0a1f366ba1ca33b61829fa16ad9 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 74c468860b42a19c29b624f6f0978e6a1ef895d3 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 97b9e261be69cd77149aca4ba20d5c628f857aef 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java a2b1d2216c69a0d983434c8c600d461d538badf7 
  src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java d5918b92a6461003772ab3d7d4440a92ba6cdd80 

Diff: https://reviews.apache.org/r/46499/diff/


Testing
-------


Thanks,

Maxim Khutornenko


Re: Review Request 46499: Revert "Moving db migration into LogStorage"

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46499/#review129932
-----------------------------------------------------------


Ship it!




Ship It!

- Joshua Cohen


On April 21, 2016, 5:08 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46499/
> -----------------------------------------------------------
> 
> (Updated April 21, 2016, 5:08 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This reverts commit ae051f3b92797d5c9f328c6c6d42d03ee4077938.
> 
> Moving the migrator call back to snapshot store.
> 
> I realized we can't rely on post-recover data migration alone to satisfy backfilling needs. During recovery, we perform various insertion calls (mem store or log storage catch up) that rely on thrift schema matching mybatis schema. This may not always be true (as we learned in AURORA-1603) and may result in incorrect or duplicate data being inserted.
> 
> The correct data migration sequence for all configuration cases should be:
> 1. migrate schema
> 2. migrate any data loaded from dbsnapshot (if applicable)
> 3. apply snapshot with backfilling
> 4. replay log store transactions with backfilling
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 5c7d92f00ddda0a1f366ba1ca33b61829fa16ad9 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 74c468860b42a19c29b624f6f0978e6a1ef895d3 
>   src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 97b9e261be69cd77149aca4ba20d5c628f857aef 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java a2b1d2216c69a0d983434c8c600d461d538badf7 
>   src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java d5918b92a6461003772ab3d7d4440a92ba6cdd80 
> 
> Diff: https://reviews.apache.org/r/46499/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 46499: Revert "Moving db migration into LogStorage"

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46499/#review129925
-----------------------------------------------------------


Ship it!




Master (15eaa5d) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On April 21, 2016, 5:08 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46499/
> -----------------------------------------------------------
> 
> (Updated April 21, 2016, 5:08 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This reverts commit ae051f3b92797d5c9f328c6c6d42d03ee4077938.
> 
> Moving the migrator call back to snapshot store.
> 
> I realized we can't rely on post-recover data migration alone to satisfy backfilling needs. During recovery, we perform various insertion calls (mem store or log storage catch up) that rely on thrift schema matching mybatis schema. This may not always be true (as we learned in AURORA-1603) and may result in incorrect or duplicate data being inserted.
> 
> The correct data migration sequence for all configuration cases should be:
> 1. migrate schema
> 2. migrate any data loaded from dbsnapshot (if applicable)
> 3. apply snapshot with backfilling
> 4. replay log store transactions with backfilling
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 5c7d92f00ddda0a1f366ba1ca33b61829fa16ad9 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 74c468860b42a19c29b624f6f0978e6a1ef895d3 
>   src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 97b9e261be69cd77149aca4ba20d5c628f857aef 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java a2b1d2216c69a0d983434c8c600d461d538badf7 
>   src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java d5918b92a6461003772ab3d7d4440a92ba6cdd80 
> 
> Diff: https://reviews.apache.org/r/46499/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>