You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by Kanak Biscuitwala <ka...@hotmail.com> on 2013/08/21 01:50:20 UTC

Review Request 13694: [HELIX-212] Rebalancer interface should have 1 function to compute the entire ideal state

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

Review request for helix, Zhen Zhang, Kishore Gopalakrishna, and Shi Lu.


Bugs: HELIX-212


Repository: helix-git


Description
-------

Fix for HELIX-212. The interface was changed to a single function and integrated into the controller stages. This led to some uncovered shortcomings in the rebalancing strategy, some of which are fixed here, and others are tracked by other issues.


Diffs
-----

  helix-core/src/main/java/org/apache/helix/HelixProperty.java e60c409 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java 2e3cce7 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java 1cbfde4 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java fc36124 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java cb8a948 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/ConstraintBasedAssignment.java dcf1f04 
  helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java 79e957c 
  helix-core/src/main/java/org/apache/helix/controller/stages/RebalanceIdealStateStage.java a049fef 
  helix-core/src/main/java/org/apache/helix/controller/stages/ResourceMapping.java dc96507 
  helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java 283e26c 
  helix-core/src/main/java/org/apache/helix/model/IdealState.java 22e2328 
  helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java PRE-CREATION 
  helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java 1d34e5b 

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


Testing
-------

Tests pass locally.


Thanks,

Kanak Biscuitwala


Re: Review Request 13694: [HELIX-212] Rebalancer interface should have 1 function to compute the entire ideal state

Posted by Kanak Biscuitwala <ka...@hotmail.com>.

> On Aug. 23, 2013, 4:38 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java, line 30
> > <https://reviews.apache.org/r/13694/diff/2/?file=343978#file343978line30>
> >
> >     Have an example here.Should we call it Assignment instead of Mapping?

Done


> On Aug. 23, 2013, 4:38 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java, line 33
> > <https://reviews.apache.org/r/13694/diff/2/?file=343978#file343978line33>
> >
> >     there are multiple methods with same signature, do u want to deprecate the one you dont need any more.

Done


> On Aug. 23, 2013, 4:38 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java, line 100
> > <https://reviews.apache.org/r/13694/diff/2/?file=343978#file343978line100>
> >
> >     kind of weird that this method exists on ResourceMapping. Probably other way round makes more sense.

Done


- Kanak


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


On Aug. 22, 2013, 10:35 p.m., Kanak Biscuitwala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13694/
> -----------------------------------------------------------
> 
> (Updated Aug. 22, 2013, 10:35 p.m.)
> 
> 
> Review request for helix, Zhen Zhang, Kishore Gopalakrishna, and Shi Lu.
> 
> 
> Bugs: HELIX-212
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> Fix for HELIX-212. The interface was changed to a single function and integrated into the controller stages. This led to some uncovered shortcomings in the rebalancing strategy, some of which are fixed here, and others are tracked by other issues.
> 
> 
> Diffs
> -----
> 
>   helix-core/src/main/java/org/apache/helix/HelixProperty.java 2e19231 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java 4dd5ea6 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java 17dc5c8 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java a0cfbb7 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java bc682ff 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/ConstraintBasedAssignment.java 3fd52f4 
>   helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java 598c318 
>   helix-core/src/main/java/org/apache/helix/controller/stages/RebalanceIdealStateStage.java cf1633c 
>   helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java 76560a4 
>   helix-core/src/main/java/org/apache/helix/model/IdealState.java b0b3960 
>   helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java PRE-CREATION 
>   helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java 7811c0d 
> 
> Diff: https://reviews.apache.org/r/13694/diff/
> 
> 
> Testing
> -------
> 
> Tests pass locally.
> 
> 
> Thanks,
> 
> Kanak Biscuitwala
> 
>


Re: Review Request 13694: [HELIX-212] Rebalancer interface should have 1 function to compute the entire ideal state

Posted by Kishore Gopalakrishna <ki...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13694/#review25457
-----------------------------------------------------------

Ship it!



helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java
<https://reviews.apache.org/r/13694/#comment49866>

    Have an example here.Should we call it Assignment instead of Mapping?



helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java
<https://reviews.apache.org/r/13694/#comment49867>

    there are multiple methods with same signature, do u want to deprecate the one you dont need any more.



helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java
<https://reviews.apache.org/r/13694/#comment49868>

    kind of weird that this method exists on ResourceMapping. Probably other way round makes more sense.


- Kishore Gopalakrishna


On Aug. 22, 2013, 10:35 p.m., Kanak Biscuitwala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13694/
> -----------------------------------------------------------
> 
> (Updated Aug. 22, 2013, 10:35 p.m.)
> 
> 
> Review request for helix, Zhen Zhang, Kishore Gopalakrishna, and Shi Lu.
> 
> 
> Bugs: HELIX-212
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> Fix for HELIX-212. The interface was changed to a single function and integrated into the controller stages. This led to some uncovered shortcomings in the rebalancing strategy, some of which are fixed here, and others are tracked by other issues.
> 
> 
> Diffs
> -----
> 
>   helix-core/src/main/java/org/apache/helix/HelixProperty.java 2e19231 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java 4dd5ea6 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java 17dc5c8 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java a0cfbb7 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java bc682ff 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/ConstraintBasedAssignment.java 3fd52f4 
>   helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java 598c318 
>   helix-core/src/main/java/org/apache/helix/controller/stages/RebalanceIdealStateStage.java cf1633c 
>   helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java 76560a4 
>   helix-core/src/main/java/org/apache/helix/model/IdealState.java b0b3960 
>   helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java PRE-CREATION 
>   helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java 7811c0d 
> 
> Diff: https://reviews.apache.org/r/13694/diff/
> 
> 
> Testing
> -------
> 
> Tests pass locally.
> 
> 
> Thanks,
> 
> Kanak Biscuitwala
> 
>


Re: Review Request 13694: [HELIX-212] Rebalancer interface should have 1 function to compute the entire ideal state

Posted by Zhen Zhang <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13694/#review25475
-----------------------------------------------------------

Ship it!


Ship It!

- Zhen Zhang


On Aug. 23, 2013, 5:50 p.m., Kanak Biscuitwala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13694/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2013, 5:50 p.m.)
> 
> 
> Review request for helix, Zhen Zhang, Kishore Gopalakrishna, and Shi Lu.
> 
> 
> Bugs: HELIX-212
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> Fix for HELIX-212. The interface was changed to a single function and integrated into the controller stages. This led to some uncovered shortcomings in the rebalancing strategy, some of which are fixed here, and others are tracked by other issues.
> 
> 
> Diffs
> -----
> 
>   helix-core/src/main/java/org/apache/helix/HelixProperty.java 2e19231 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java 4dd5ea6 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java 17dc5c8 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java a0cfbb7 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java bc682ff 
>   helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/ConstraintBasedAssignment.java 3fd52f4 
>   helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java 598c318 
>   helix-core/src/main/java/org/apache/helix/controller/stages/RebalanceIdealStateStage.java cf1633c 
>   helix-core/src/main/java/org/apache/helix/controller/stages/ResourceMapping.java 2609791 
>   helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java 76560a4 
>   helix-core/src/main/java/org/apache/helix/model/IdealState.java 463369a 
>   helix-core/src/main/java/org/apache/helix/model/ResourceAssignment.java b0d7f1f 
>   helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java 7811c0d 
> 
> Diff: https://reviews.apache.org/r/13694/diff/
> 
> 
> Testing
> -------
> 
> Tests pass locally.
> 
> 
> Thanks,
> 
> Kanak Biscuitwala
> 
>


Re: Review Request 13694: [HELIX-212] Rebalancer interface should have 1 function to compute the entire ideal state

Posted by Kanak Biscuitwala <ka...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13694/
-----------------------------------------------------------

(Updated Aug. 23, 2013, 5:50 p.m.)


Review request for helix, Zhen Zhang, Kishore Gopalakrishna, and Shi Lu.


Bugs: HELIX-212


Repository: helix-git


Description
-------

Fix for HELIX-212. The interface was changed to a single function and integrated into the controller stages. This led to some uncovered shortcomings in the rebalancing strategy, some of which are fixed here, and others are tracked by other issues.


Diffs (updated)
-----

  helix-core/src/main/java/org/apache/helix/HelixProperty.java 2e19231 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java 4dd5ea6 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java 17dc5c8 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java a0cfbb7 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java bc682ff 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/ConstraintBasedAssignment.java 3fd52f4 
  helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java 598c318 
  helix-core/src/main/java/org/apache/helix/controller/stages/RebalanceIdealStateStage.java cf1633c 
  helix-core/src/main/java/org/apache/helix/controller/stages/ResourceMapping.java 2609791 
  helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java 76560a4 
  helix-core/src/main/java/org/apache/helix/model/IdealState.java 463369a 
  helix-core/src/main/java/org/apache/helix/model/ResourceAssignment.java b0d7f1f 
  helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java 7811c0d 

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


Testing
-------

Tests pass locally.


Thanks,

Kanak Biscuitwala


Re: Review Request 13694: [HELIX-212] Rebalancer interface should have 1 function to compute the entire ideal state

Posted by Kanak Biscuitwala <ka...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13694/
-----------------------------------------------------------

(Updated Aug. 22, 2013, 10:35 p.m.)


Review request for helix, Zhen Zhang, Kishore Gopalakrishna, and Shi Lu.


Changes
-------

Removed some additional dead code.


Bugs: HELIX-212


Repository: helix-git


Description
-------

Fix for HELIX-212. The interface was changed to a single function and integrated into the controller stages. This led to some uncovered shortcomings in the rebalancing strategy, some of which are fixed here, and others are tracked by other issues.


Diffs (updated)
-----

  helix-core/src/main/java/org/apache/helix/HelixProperty.java 2e19231 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java 4dd5ea6 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java 17dc5c8 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java a0cfbb7 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java bc682ff 
  helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/ConstraintBasedAssignment.java 3fd52f4 
  helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java 598c318 
  helix-core/src/main/java/org/apache/helix/controller/stages/RebalanceIdealStateStage.java cf1633c 
  helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java 76560a4 
  helix-core/src/main/java/org/apache/helix/model/IdealState.java b0b3960 
  helix-core/src/main/java/org/apache/helix/model/ResourceMapping.java PRE-CREATION 
  helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java 7811c0d 

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


Testing
-------

Tests pass locally.


Thanks,

Kanak Biscuitwala