----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13390/#review24847 -----------------------------------------------------------
Ship it! Can you see if the test coverage increase/decrease after this change helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java <https://reviews.apache.org/r/13390/#comment49006> javadoc for this, give an example input/output helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java <https://reviews.apache.org/r/13390/#comment49007> remove this line, helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java <https://reviews.apache.org/r/13390/#comment49008> Having a map<string,map<String,string> is confusing. Have a new class called ResourceMapping or something like that helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java <https://reviews.apache.org/r/13390/#comment49009> java doc helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java <https://reviews.apache.org/r/13390/#comment49010> remove this line helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java <https://reviews.apache.org/r/13390/#comment49011> see the OSGi patch. Using class.forname is not liked by OSGi helix-core/src/main/java/org/apache/helix/controller/stages/util/BestStateCalculator.java <https://reviews.apache.org/r/13390/#comment49012> should probably be under rebalancer.util. This is kind of the constraint solver right. need a better name. helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java <https://reviews.apache.org/r/13390/#comment49013> javadoc - Kishore Gopalakrishna On Aug. 7, 2013, 11:26 p.m., Kanak Biscuitwala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13390/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2013, 11:26 p.m.) > > > Review request for helix, Zhen Zhang, Kishore Gopalakrishna, and Shi Lu. > > > Bugs: HELIX-173 > > > Repository: helix-git > > > Description > ------- > > Fix for HELIX-173. Right now, the code to compute mappings from partitions to > nodes and states is mostly in a single file, regardless of the rebalancing > mode. Moving these operations cleans up the code and is more logical for > pluggable rebalancers. > > > Diffs > ----- > > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java > 62a73b3 > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java > aca0e74 > > helix-core/src/main/java/org/apache/helix/controller/stages/util/BestStateCalculator.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java > f013937 > > helix-core/src/test/java/org/apache/helix/controller/strategy/TestAutoRebalanceStrategy.java > 50a2f81 > > helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java > df32c4e > > Diff: https://reviews.apache.org/r/13390/diff/ > > > Testing > ------- > > Tests pass locally. > > > Thanks, > > Kanak Biscuitwala > >
