Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70680 --- Ship it! Ship It! - Maxim Khutornenko On Feb. 3, 2015, 12:42 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 3, 2015, 12:42 a.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
On Feb. 3, 2015, 12:48 a.m., Maxim Khutornenko wrote: The ticket suggests a possibility of the optimization route. Would you mind commenting why you decided to remove it after all? Bill Farner wrote: Sure. Kevin rightly pointed out that it's odd for us to check this _one_ invariant when there are many others. Put another way, we can't check everything, and doing this one check suggests that we are uncertain of our ability to maintain integrity of the data in this one specific way. Once we move this table to SQL, we will be in a much better position to continually enforce this type of constraint. SGTM. Thanks for explaining. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70675 --- On Feb. 3, 2015, 12:42 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 3, 2015, 12:42 a.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70675 --- The ticket suggests a possibility of the optimization route. Would you mind commenting why you decided to remove it after all? - Maxim Khutornenko On Feb. 3, 2015, 12:42 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 3, 2015, 12:42 a.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70671 --- Ship it! Ship It! - Kevin Sweeney On Feb. 2, 2015, 4:42 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 2, 2015, 4:42 p.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
On Feb. 3, 2015, 12:48 a.m., Maxim Khutornenko wrote: The ticket suggests a possibility of the optimization route. Would you mind commenting why you decided to remove it after all? Sure. Kevin rightly pointed out that it's odd for us to check this _one_ invariant when there are many others. Put another way, we can't check everything, and doing this one check suggests that we are uncertain of our ability to maintain integrity of the data in this one specific way. Once we move this table to SQL, we will be in a much better position to continually enforce this type of constraint. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70675 --- On Feb. 3, 2015, 12:42 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 3, 2015, 12:42 a.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70684 --- Ship it! Master (a674581) 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. 3, 2015, 12:42 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 3, 2015, 12:42 a.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
Could the impact of this change be verified by our performance benchmarks? On Mon, Feb 2, 2015 at 5:06 PM, Aurora ReviewBot wfar...@apache.org wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70684 --- Ship it! Master (a674581) 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. 3, 2015, 12:42 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 3, 2015, 12:42 a.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner -- Zameer Manji
Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
+1 to moving forward to this patch as-is On Mon, Feb 2, 2015 at 5:39 PM, Bill Farner wfar...@apache.org wrote: They could, but I don't think it would be worth the effort here. In this case, the code itself is of little value, it's a bonus that it is also a performance hog. On Monday, February 2, 2015, Zameer Manji zma...@twopensource.com wrote: Could the impact of this change be verified by our performance benchmarks? On Mon, Feb 2, 2015 at 5:06 PM, Aurora ReviewBot wfar...@apache.org wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70684 --- Ship it! Master (a674581) 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. 3, 2015, 12:42 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 3, 2015, 12:42 a.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner -- Zameer Manji -- -=Bill
Re: Review Request 30535: Remove shard uniqueness check from scheduler recovery phase.
They could, but I don't think it would be worth the effort here. In this case, the code itself is of little value, it's a bonus that it is also a performance hog. On Monday, February 2, 2015, Zameer Manji zma...@twopensource.com wrote: Could the impact of this change be verified by our performance benchmarks? On Mon, Feb 2, 2015 at 5:06 PM, Aurora ReviewBot wfar...@apache.org javascript:_e(%7B%7D,'cvml','wfar...@apache.org'); wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/#review70684 --- Ship it! Master (a674581) 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. 3, 2015, 12:42 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30535/ --- (Updated Feb. 3, 2015, 12:42 a.m.) Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-1090 https://issues.apache.org/jira/browse/AURORA-1090 Repository: aurora Description --- Remove shard uniqueness check from scheduler recovery phase. Diffs - src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 1814658c044273f7c3a2348a16aea62e397cf860 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 93773eb5ba3bee1b3296e69ea30eabb531eeb661 Diff: https://reviews.apache.org/r/30535/diff/ Testing --- Thanks, Bill Farner -- Zameer Manji -- -=Bill
