[go: up one dir, main page]

JavaScript is turned off

Review Board requires JavaScript in order to function. Please turn it on in your browser preferences.

Firefox users: if you prefer to turn on JavaScript only for specific sites, we recommend the NoScript extension.

[HELIX-543] Avoid moving partitions unnecessarily when auto-rebalancing

Review Request #27808 - Created Nov. 10, 2014 and updated

Information
Tom Widmer
helix-git
HELIX-543
Reviewers
helix
kanak

commit ba45d4bc2019c4069fe810c6e61a0696391f207e
Author: Tom Widmer <tom.widmer@camcog.com>
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 <tom.widmer@camcog.com>
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

Added new test to check case (failed before, now passes). Ran other re-balancing tests.

Kishore Gopalakrishna

Nice work!. Thanks for identifying this. Will be great to have a integration test. I can add that after we push this code. replicaCount is using String as key that is resulting in state.tostring, it might be better to use the concrete type(state) instead of converting it to String. We should probably fix that ((not neccessarily part of this review/jira))

Kanak Biscuitwala

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.

thanks!

active for each participant*

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

Loading...