add delete cluster setting and throw exception when validation fails#5019
add delete cluster setting and throw exception when validation fails#5019kzwang wants to merge 2 commits intoelastic:masterfrom
Conversation
|
👍 |
|
hey @kzwang I can't tell you how much I'd love to get this in - I wanted to use it for our test but it's a bit more tricky I think than it looks like. It's not enough to delete the setting we really need to reset to what it's default was. for instance look at class ApplySettings implements NodeSettingsService.Listener {
@Override
public void onRefreshSettings(Settings settings) {
int clusterConcurrentRebalance = settings.getAsInt(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance);
if (clusterConcurrentRebalance != ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance) {
logger.info("updating [cluster.routing.allocation.cluster_concurrent_rebalance] from [{}], to [{}]", ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance, clusterConcurrentRebalance);
ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance = clusterConcurrentRebalance;
}
}
}once the setting was applied it sticks to where it is used and will be reused even if we delete it here is the relevant line: int clusterConcurrentRebalance = settings.getAsInt(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance);so we need to find a way to reset it to the default but I don't have a really good answer how to do that... |
|
Hi @s1monw, |
|
so I think this can work but it's tricky. What we need to do is this this.clusterConcurrentRebalance = this.defaultClusterConcurrentRebalance = settings.getAsInt(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, 2);
//... and then
int clusterConcurrentRebalance = settings.getAsInt(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, ConcurrentRebalanceAllocationDecider.this.defaultClusterConcurrentRebalance);since the constant might not be the default that comes from the node setting. I personally think a prerequsite for this is a way to fetch the current value that is used for each of the settings so we can actually test the update / deletes. does this make sense? |
|
one way of doing this could be adding something like this: class ApplySettings implements NodeSettingsService.Listener {
public void currentSettings(ImmutableSettings.Builder builder) {
builder.put(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance);
}
@Override
public void onRefreshSettings(Settings settings) {
int clusterConcurrentRebalance = settings.getAsInt(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance);
if (clusterConcurrentRebalance != ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance) {
logger.info("updating [cluster.routing.allocation.cluster_concurrent_rebalance] from [{}], to [{}]", ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance, clusterConcurrentRebalance);
ConcurrentRebalanceAllocationDecider.this.clusterConcurrentRebalance = clusterConcurrentRebalance;
}
}
}where we can pull the current settings from? |
|
what I'm thinking is the default value in code is just a static field that should never changed, and we'll pass all settings (cluster wide, node setting from config file etc.) to |
|
well that is not true - the values might be configured on a node level in the |
private static final int defaultClusterConcurrentRebalance = 2;
// in constructor
int clusterConcurrentRebalance = settings.getAsInt(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, ConcurrentRebalanceAllocationDecider.this.defaultClusterConcurrentRebalance);
// adn change this method to read both cluster setting and node setting
@Override
public void onRefreshSettings(Settings settings, Settings nodeSettings) {
int clusterConcurrentRebalance = settings.getAsInt(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, nodeSettings.getAsInt(CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE, ConcurrentRebalanceAllocationDecider.this.defaultClusterConcurrentRebalance));
//...
}when construct the class, it will read node setting, when setting updated, it will read cluster setting first, if not found, try node setting, if still not found, use the static default one |
|
oh yeah that is a great idea - I like it. If we can pair it with getting the effective value via a GET call that would be awesome! |
|
@kzwang would you be willing to work on this further? If you don't have time we can find somebody to take it from here? |
|
@s1monw I'll work on it within next couple of days |
|
awesome! thanks so much! |
|
I am sure glad to see this is being looked at, a much needed feature - especially with deprecated settings in the mix! |
|
Closed in favour of #6732 |
Added
DELETE,POSTand modifiedPUTmethod for "/_cluster/settings"DELETErequest will delete transient and persistent settings, there are two parametersdelete_transientanddelete_persistentto control delete transient and persistent settings, they are all default to true. It will return the deleted settingsPOSTwill add the settings in request to the cluster settings (same as previousPUT)PUTwill clear the existing settings and add the settings in the requestAlso will throw exception when validation for settings fail
cloeses #3670 and #5018