----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27808/#review60959 -----------------------------------------------------------
Ship it! Looks good, assuming that existing integration tests still pass. Some ones to try include TestAutoRebalance, TestFullAutoNodeTagging, TestAutoRebalancePartitionLimit, and TestIndependentTaskRebalancer. Only minor comments regarding the code. helix-agent/helix-agent-0.7.2-SNAPSHOT.ivy <https://reviews.apache.org/r/27808/#comment102379> thanks! helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java <https://reviews.apache.org/r/27808/#comment102380> active for each participant* helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java <https://reviews.apache.org/r/27808/#comment102381> It may be cleaner to use Guava comparator helpers, but this is fine too. Reference: https://code.google.com/p/guava-libraries/wiki/CommonObjectUtilitiesExplained#compare/compareTo - Kanak Biscuitwala On Nov. 10, 2014, 5:07 a.m., Tom Widmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27808/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2014, 5:07 a.m.) > > > Review request for helix and Kanak Biscuitwala. > > > Bugs: HELIX-543 > > > Repository: helix-git > > > Description > ------- > > commit ba45d4bc2019c4069fe810c6e61a0696391f207e > Author: Tom Widmer <[email protected]> > Date: Mon Nov 10 12:40:33 2014 +0000 > > [HELIX-543] Add missing licence header > > :100644 100644 f59be07... ef1f57e... M > helix-agent/helix-agent-0.7.2-SNAPSHOT.ivy > > commit 5f63c6d69d594c70e85de31d5ed67f62f348ecb0 > Author: Tom Widmer <[email protected]> > Date: Mon Nov 10 12:26:40 2014 +0000 > > [HELIX-543] Avoid moving partitions unnecessarily when auto-rebalancing > > This is done by allocating capacity first to those nodes which already > have > the most partitions. > > :100644 100644 09b66c1... 6e0e226... M > helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java > :100644 100644 1322b40... 25c550d... M > helix-core/src/test/java/org/apache/helix/controller/strategy/TestAutoRebalanceStrategy.java > > > Diffs > ----- > > helix-agent/helix-agent-0.7.2-SNAPSHOT.ivy f59be07 > > helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java > 09b66c1 > > helix-core/src/test/java/org/apache/helix/controller/strategy/TestAutoRebalanceStrategy.java > 1322b40 > > Diff: https://reviews.apache.org/r/27808/diff/ > > > Testing > ------- > > Added new test to check case (failed before, now passes). Ran other > re-balancing tests. > > > Thanks, > > Tom Widmer > >
