You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2015/02/03 01:42:19 UTC

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/
-----------------------------------------------------------

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.

Posted by Maxim Khutornenko <ma...@apache.org>.

> 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.

Posted by Bill Farner <wf...@apache.org>.

> 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.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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.

Posted by Kevin Sweeney <ke...@apache.org>.
+1 to moving forward to this patch as-is

On Mon, Feb 2, 2015 at 5:39 PM, Bill Farner <wf...@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 <zm...@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 <wf...@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.

Posted by Bill Farner <wf...@apache.org>.
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 <zm...@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 <wfarner@apache.org
> <javascript:_e(%7B%7D,'cvml','wfarner@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.

Posted by Zameer Manji <zm...@twopensource.com>.
Could the impact of this change be verified by our performance benchmarks?

On Mon, Feb 2, 2015 at 5:06 PM, Aurora ReviewBot <wf...@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.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
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
> 
>