Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed improvements for BalancedShardsAllocator #15678

Merged
merged 11 commits into from Dec 28, 2015

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Dec 28, 2015

This PR contains some speed improvements for BalancedShardsAllocator (relates to #6372):

  • Removal of the pattern node.addShard() -> calculate weight -> node.removeShard() which is expensive as, beside map lookups, it invalidates caching of precomputed values in ModelNode and ModelIndex. Replaced by adding an additional parameter to the weight function which accounts for the added / removed shard.
  • For rebalancing an index, only consider nodes that currently have a shard of that index or where the index can be allocated. This allows to prune a large number of nodes in case of hot/warm setup. Implemented by adding allocation decider rule based on IndexMetaData.
  • Removed some dead code
  • Some caching (avoid reresolving values in map all the time) / simplification of code

I made smaller commits for each improvement to simplify the review process.

final float weightIndex = (node.numShards(index) - balancer.avgShardsPerNode(index));
assert theta != null;
return theta[0] * weightShard + theta[1] * weightIndex;
public float weight(Balancer balancer, ModelNode node, String index, int numAdditionalShards) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we instead of exposing this additional parameter just have some dedicated methods for +1, -1, 0 and keep that method internal?

@s1monw
Copy link
Contributor

s1monw commented Dec 28, 2015

left one comment - LGTM otherwise

Yannick Welsch added 11 commits December 28, 2015 19:14
…emoved shard

Removal of the pattern node.addShard() -> calculate weight -> node.removeShard() which is expensive as, beside map lookups, it invalidates caching of precomputed values in ModelNode and ModelIndex. Replaced by adding an additional parameter to the weight function which accounts for the added / removed shard.
…hard of that index or where the index can be allocated

This allows to prune a large number of nodes in case of hot/warm setup
@ywelsch
Copy link
Contributor Author

ywelsch commented Dec 28, 2015

Addressed comment and added labels. Thanks for the review!

ywelsch pushed a commit that referenced this pull request Dec 28, 2015
Speed improvements for BalancedShardsAllocator
@ywelsch ywelsch merged commit 24ab66c into elastic:master Dec 28, 2015
@clintongormley
Copy link

Nice!

@lcawl lcawl added :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. and removed :Allocation labels Feb 13, 2018
@clintongormley clintongormley added :Distributed/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) and removed :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) >enhancement v2.3.0 v5.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants