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

Add number_of_routing_shards config set to 30 #5570

Merged
merged 1 commit into from
Nov 17, 2017

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Nov 14, 2017

With elastic/elasticsearch#26931 the possibility for splitting shards was introduced. To make use of this feature for indices created with ES >=6.1 the config option index.number_of_routing_shards is required. This adds this config option currently set to 30 as it's a multiple of 1, 3 and 5, our current number of default shards in Beats and ES. This allows users with default configs to scale their split their shards.

The number_of_routing_shards can also be overwritten in the config file.

@ruflin ruflin force-pushed the number-of-routing-shards branch 2 times, most recently from 6eb64e0 to fd1c914 Compare November 14, 2017 03:16
@tsg
Copy link
Contributor

tsg commented Nov 14, 2017

The tests complain that:

could not load template: couldn't load template: couldn't load json. Error: 400 Bad Request: {\"error\":{\"root_cause\":[{\"type\":\"illegal_argument_exception\",\"reason\":\"the number of source shards [5] must be a must be a factor of [24]\"}],\"type\":\"illegal_argument_exception\",\"reason\":\"the number of source shards [5] must be a must be a factor of [24]\"},\"status\":400}.

I think that's currently only an issue in tests, but it does worry me that if anyone is changing number_of_shards to 5 in their Beat configuration, they will hit that error.

I think we should:

  • Make the number_of_routing_shards also a configuration option in the Beat config file, if it's not already there indirectly.
  • Change the number to include 5? Maybe 2 * 3 * 5 = 30? Or 4 * 3 * 5 = 60?

@ruflin
Copy link
Contributor Author

ruflin commented Nov 15, 2017

Setting it to 30, would mean for the once with 1 shard could do 1,2,3,5,6,10,15,30 the once with 3 shards could do 6,15,30 and 5 shards could do 10,15,30. Going with 60 would only add 12 to the 3 shard option in the lower band, so not a big benefit. So I think 30 would be a pretty good pick also with the ones that have the default shards set to 5. I think the majority of people will use shards in a single digit area.

@ruflin ruflin added the in progress Pull request is currently in progress. label Nov 15, 2017
@ruflin ruflin force-pushed the number-of-routing-shards branch from fd1c914 to f115314 Compare November 15, 2017 05:10
@ruflin ruflin changed the title Add number_of_routing_shards config set to 24 Add number_of_routing_shards config set to 30 Nov 16, 2017
@ruflin ruflin force-pushed the number-of-routing-shards branch from f115314 to 64c4447 Compare November 16, 2017 05:30
With elastic/elasticsearch#26931 the possibility for splitting shards was introduced. To make use of this feature for indices created with ES >=6.1 the config option `index.number_of_routing_shards` is required. This adds this config option currently set to 30 as it's a multiple of 1, 3 and 5, our current number of default shards in Beats and ES. This allows users with default configs to scale their split their shards.

The `number_of_routing_shards` can also be overwritten in the config file.
@ruflin ruflin force-pushed the number-of-routing-shards branch from 64c4447 to 1edbd77 Compare November 17, 2017 02:23
@ruflin ruflin removed the in progress Pull request is currently in progress. label Nov 17, 2017
@ruflin
Copy link
Contributor Author

ruflin commented Nov 17, 2017

@tsg Can you take a look again. I update it to be 30 by default and also updated PR title and description accordingly. I added it to the reference config and added a test to checked if it can be overwritten.

Copy link
Contributor

@tsg tsg left a comment

Choose a reason for hiding this comment

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

LGTM.

@tsg tsg merged commit 75ad4f6 into elastic:master Nov 17, 2017
@ruflin ruflin deleted the number-of-routing-shards branch January 4, 2018 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants