Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.

2015-02-24 Thread Bill Farner 


> On Feb. 24, 2015, 8:54 p.m., Zameer Manji wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml,
> >  line 149
> > 
> >
> > Have you filed an upstream bug about this behaviour?
> 
> Bill Farner wrote:
> I ventured a search at whether this is by design, i welcome your 
> contribution.  Tracing the mybatis code for why exactly this happens is not 
> how i would like to spend my afternoon, and i do not want to file an ignorant 
> bug report.

https://issues.apache.org/jira/browse/AURORA-1142


- Bill


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


On Feb. 24, 2015, 8:12 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31376/
> ---
> 
> (Updated Feb. 24, 2015, 8:12 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are 3 ways a JobUpdateSummary lands in storage:
> 
> - starting a job update via thrift
> - replaying a log record to save a job update
> - replaying a snapshot log record
> 
> This change focuses on ensuring that `JobUpdateSummary.key` is always 
> available for code that is currently relying on the legacy fields (`updateId` 
> and `jobKey`) by cloning them.
> 
> A follow-up change will alter all code inside the scheduler to only consume 
> the new field (removing `Updates.getKey`).
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7685597bb6acafe1412b23234227adb0eddad429 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 
> 
> Diff: https://reviews.apache.org/r/31376/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.

2015-02-24 Thread Maxim Khutornenko 


> On Feb. 24, 2015, 8:54 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/Updates.java, line 57
> > 
> >
> > I don't think we should pass around any mutable state. I think data 
> > objects like JobUpdateSummary should be immutable and the extra complextity 
> > at call sites is worth the safety we get.

I agree wrt permanent changes outside of backfill logic. This, however, is a 
short-term migration hack that is not supposed to be used anywhere outside of 
backfilling.

Anyway, I am ok with the change. Just feels weird to go through all these hoops 
in this particular case.


- Maxim


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


On Feb. 24, 2015, 8:12 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31376/
> ---
> 
> (Updated Feb. 24, 2015, 8:12 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are 3 ways a JobUpdateSummary lands in storage:
> 
> - starting a job update via thrift
> - replaying a log record to save a job update
> - replaying a snapshot log record
> 
> This change focuses on ensuring that `JobUpdateSummary.key` is always 
> available for code that is currently relying on the legacy fields (`updateId` 
> and `jobKey`) by cloning them.
> 
> A follow-up change will alter all code inside the scheduler to only consume 
> the new field (removing `Updates.getKey`).
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7685597bb6acafe1412b23234227adb0eddad429 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 
> 
> Diff: https://reviews.apache.org/r/31376/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.

2015-02-24 Thread Bill Farner 


> On Feb. 24, 2015, 8:54 p.m., Zameer Manji wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml,
> >  line 149
> > 
> >
> > Have you filed an upstream bug about this behaviour?

I ventured a search at whether this is by design, i welcome your contribution.  
Tracing the mybatis code for why exactly this happens is not how i would like 
to spend my afternoon, and i do not want to file an ignorant bug report.


- Bill


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


On Feb. 24, 2015, 8:12 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31376/
> ---
> 
> (Updated Feb. 24, 2015, 8:12 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are 3 ways a JobUpdateSummary lands in storage:
> 
> - starting a job update via thrift
> - replaying a log record to save a job update
> - replaying a snapshot log record
> 
> This change focuses on ensuring that `JobUpdateSummary.key` is always 
> available for code that is currently relying on the legacy fields (`updateId` 
> and `jobKey`) by cloning them.
> 
> A follow-up change will alter all code inside the scheduler to only consume 
> the new field (removing `Updates.getKey`).
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7685597bb6acafe1412b23234227adb0eddad429 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 
> 
> Diff: https://reviews.apache.org/r/31376/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.

2015-02-24 Thread Zameer Manji 

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



src/main/java/org/apache/aurora/scheduler/updater/Updates.java


I don't think we should pass around any mutable state. I think data objects 
like JobUpdateSummary should be immutable and the extra complextity at call 
sites is worth the safety we get.



src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml


Have you filed an upstream bug about this behaviour?


- Zameer Manji


On Feb. 24, 2015, 12:12 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31376/
> ---
> 
> (Updated Feb. 24, 2015, 12:12 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are 3 ways a JobUpdateSummary lands in storage:
> 
> - starting a job update via thrift
> - replaying a log record to save a job update
> - replaying a snapshot log record
> 
> This change focuses on ensuring that `JobUpdateSummary.key` is always 
> available for code that is currently relying on the legacy fields (`updateId` 
> and `jobKey`) by cloning them.
> 
> A follow-up change will alter all code inside the scheduler to only consume 
> the new field (removing `Updates.getKey`).
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7685597bb6acafe1412b23234227adb0eddad429 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 
> 
> Diff: https://reviews.apache.org/r/31376/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.

2015-02-24 Thread Zameer Manji 

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

Ship it!


Ship It!

- Zameer Manji


On Feb. 24, 2015, 12:12 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31376/
> ---
> 
> (Updated Feb. 24, 2015, 12:12 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are 3 ways a JobUpdateSummary lands in storage:
> 
> - starting a job update via thrift
> - replaying a log record to save a job update
> - replaying a snapshot log record
> 
> This change focuses on ensuring that `JobUpdateSummary.key` is always 
> available for code that is currently relying on the legacy fields (`updateId` 
> and `jobKey`) by cloning them.
> 
> A follow-up change will alter all code inside the scheduler to only consume 
> the new field (removing `Updates.getKey`).
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7685597bb6acafe1412b23234227adb0eddad429 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 
> 
> Diff: https://reviews.apache.org/r/31376/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.

2015-02-24 Thread Bill Farner 


> On Feb. 24, 2015, 8:28 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/Updates.java, line 57
> > 
> >
> > Would it make sense to accept/return `JobUpdateSummary`? The 
> > mutable/immutable dance is rather confusing at call sites.

I'm generally weary of passing mutable state around.  I feel that this 
signature more clearly communicates the behavior than accepting a mutable value 
would.

Interested in what Zameer thinks for a third opinion, though.


- Bill


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


On Feb. 24, 2015, 8:12 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31376/
> ---
> 
> (Updated Feb. 24, 2015, 8:12 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are 3 ways a JobUpdateSummary lands in storage:
> 
> - starting a job update via thrift
> - replaying a log record to save a job update
> - replaying a snapshot log record
> 
> This change focuses on ensuring that `JobUpdateSummary.key` is always 
> available for code that is currently relying on the legacy fields (`updateId` 
> and `jobKey`) by cloning them.
> 
> A follow-up change will alter all code inside the scheduler to only consume 
> the new field (removing `Updates.getKey`).
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7685597bb6acafe1412b23234227adb0eddad429 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 
> 
> Diff: https://reviews.apache.org/r/31376/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.

2015-02-24 Thread Aurora ReviewBot 

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

Ship it!


Master (19378c1) 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 Feb. 24, 2015, 8:12 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31376/
> ---
> 
> (Updated Feb. 24, 2015, 8:12 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are 3 ways a JobUpdateSummary lands in storage:
> 
> - starting a job update via thrift
> - replaying a log record to save a job update
> - replaying a snapshot log record
> 
> This change focuses on ensuring that `JobUpdateSummary.key` is always 
> available for code that is currently relying on the legacy fields (`updateId` 
> and `jobKey`) by cloning them.
> 
> A follow-up change will alter all code inside the scheduler to only consume 
> the new field (removing `Updates.getKey`).
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7685597bb6acafe1412b23234227adb0eddad429 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 
> 
> Diff: https://reviews.apache.org/r/31376/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.

2015-02-24 Thread Maxim Khutornenko 

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

Ship it!



src/main/java/org/apache/aurora/scheduler/updater/Updates.java


Would it make sense to accept/return `JobUpdateSummary`? The 
mutable/immutable dance is rather confusing at call sites.


- Maxim Khutornenko


On Feb. 24, 2015, 8:12 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31376/
> ---
> 
> (Updated Feb. 24, 2015, 8:12 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1093
> https://issues.apache.org/jira/browse/AURORA-1093
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are 3 ways a JobUpdateSummary lands in storage:
> 
> - starting a job update via thrift
> - replaying a log record to save a job update
> - replaying a snapshot log record
> 
> This change focuses on ensuring that `JobUpdateSummary.key` is always 
> available for code that is currently relying on the legacy fields (`updateId` 
> and `jobKey`) by cloning them.
> 
> A follow-up change will alter all code inside the scheduler to only consume 
> the new field (removing `Updates.getKey`).
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 6f6124a4c844a1abcf07401d80c3e50eb8b4 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/main/java/org/apache/aurora/scheduler/updater/Updates.java 
> 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  7685597bb6acafe1412b23234227adb0eddad429 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 
> 
> Diff: https://reviews.apache.org/r/31376/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>