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

Move cluster.routing.allocation.same_shard.host setting to new settings infrastructure #20046

Merged
merged 1 commit into from Aug 23, 2016

Conversation

masaruh
Copy link
Contributor

@masaruh masaruh commented Aug 18, 2016

Fixes #20045

@clintongormley clintongormley changed the title Move cluster.routing.allocation.same_shard.host setting to new settin… Move cluster.routing.allocation.same_shard.host setting to new settings infrastructure Aug 18, 2016
public static final String SAME_HOST_SETTING = "cluster.routing.allocation.same_shard.host";
public static final Setting<Boolean> CLUSTER_ROUTING_ALLOCATION_SAME_HOST_SETTING =
Setting.boolSetting("cluster.routing.allocation.same_shard.host", false,
Setting.Property.Dynamic, Setting.Property.NodeScope);
Copy link
Member

Choose a reason for hiding this comment

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

I'm unsure if this setting should be dynamic.

Copy link
Contributor

Choose a reason for hiding this comment

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

As this setting should usually be only set once, it is probably simpler to leave it non-dynamic (as @jasontedor suggested and as it was before this PR). In case where this must absolutely be updated on a production cluster, rolling restart (of master nodes) with config update is always possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I wondered if it should be dynamic or not.
The reason I made it dynamic is document says it's dynamic.
(and thought making it dynamic maybe useful)

I'll make it non-dynamic.

@masaruh
Copy link
Contributor Author

masaruh commented Aug 22, 2016

@jasontedor @ywelsch Made it static (and rebased). Can you review it again?

@s1monw
Copy link
Contributor

s1monw commented Aug 22, 2016

LGTM

@masaruh masaruh merged commit f3cddef into elastic:master Aug 23, 2016
@masaruh masaruh deleted the same_shard_host_setting branch August 23, 2016 02:35
@masaruh
Copy link
Contributor Author

masaruh commented Aug 23, 2016

Thanks @s1monw.

@masaruh masaruh removed the review label Aug 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Settings Settings infrastructure and APIs v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants