diff --git a/server/src/internalClusterTest/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java index 5a557b57874..a727eea1efc 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java @@ -21,10 +21,13 @@ package org.elasticsearch.indices.settings; import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.IndexModule; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.VersionType; @@ -686,4 +689,52 @@ public class UpdateSettingsIT extends ESIntegTestCase { assertThat(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(response.getIndexToSettings().get("test")), equalTo(1)); } + public void testNoopUpdate() { + internalCluster().ensureAtLeastNumDataNodes(2); + final ClusterService clusterService = internalCluster().getMasterNodeInstance(ClusterService.class); + assertAcked(client().admin().indices().prepareCreate("test") + .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0))); + + ClusterState currentState = clusterService.state(); + assertAcked(client().admin().indices().prepareUpdateSettings("test") + .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1))); + assertNotSame(currentState, clusterService.state()); + client().admin().cluster().prepareHealth() + .setWaitForGreenStatus() + .setWaitForNoInitializingShards(true) + .setWaitForEvents(Priority.LANGUID) + .setTimeout(TimeValue.MAX_VALUE) + .get(); + currentState = clusterService.state(); + + assertAcked(client().admin().indices().prepareUpdateSettings("test") + .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1))); + assertSame(clusterService.state(), currentState); + + assertAcked(client().admin().indices().prepareUpdateSettings("test") + .setSettings(Settings.builder().putNull(IndexMetadata.SETTING_NUMBER_OF_REPLICAS))); + assertSame(clusterService.state(), currentState); + + assertAcked(client().admin().indices().prepareUpdateSettings("test") + .setSettings(Settings.builder() + .putNull(SETTING_BLOCKS_READ) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1))); + assertSame(currentState, clusterService.state()); + + assertAcked(client().admin().indices().prepareUpdateSettings("test") + .setSettings(Settings.builder() + .put(SETTING_BLOCKS_READ, true) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1))); + assertNotSame(currentState, clusterService.state()); + currentState = clusterService.state(); + + assertAcked(client().admin().indices().prepareUpdateSettings("test") + .setSettings(Settings.builder().put(SETTING_BLOCKS_READ, true))); + assertSame(currentState, clusterService.state()); + + assertAcked(client().admin().indices().prepareUpdateSettings("test") + .setSettings(Settings.builder().putNull(SETTING_BLOCKS_READ))); + assertNotSame(currentState, clusterService.state()); + } + } diff --git a/server/src/main/java/org/elasticsearch/cluster/block/ClusterBlocks.java b/server/src/main/java/org/elasticsearch/cluster/block/ClusterBlocks.java index 2f35736e052..91b3e6fff41 100644 --- a/server/src/main/java/org/elasticsearch/cluster/block/ClusterBlocks.java +++ b/server/src/main/java/org/elasticsearch/cluster/block/ClusterBlocks.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.rest.RestStatus; import java.io.IOException; +import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; import java.util.HashSet; @@ -405,6 +406,10 @@ public class ClusterBlocks extends AbstractDiffable { return this; } + public boolean hasIndexBlock(String index, ClusterBlock block) { + return indices.getOrDefault(index, Collections.emptySet()).contains(block); + } + public Builder removeIndexBlock(String index, ClusterBlock block) { if (!indices.containsKey(index)) { return this; diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java index a308af92223..32bb85f3f67 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java @@ -177,11 +177,6 @@ public class MetadataUpdateSettingsService { } } - ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks()); - for (IndexMetadata.APIBlock block : IndexMetadata.APIBlock.values()) { - maybeUpdateClusterBlock(actualIndices, blocks, block.block, block.setting, openSettings); - } - if (!openIndices.isEmpty()) { for (Index index : openIndices) { IndexMetadata indexMetadata = metadataBuilder.getSafe(index); @@ -246,15 +241,26 @@ public class MetadataUpdateSettingsService { MetadataCreateIndexService.validateTranslogRetentionSettings(metadataBuilder.get(index).getSettings()); } } + boolean changed = false; // increment settings versions for (final String index : actualIndices) { if (same(currentState.metadata().index(index).getSettings(), metadataBuilder.get(index).getSettings()) == false) { + changed = true; final IndexMetadata.Builder builder = IndexMetadata.builder(metadataBuilder.get(index)); builder.settingsVersion(1 + builder.settingsVersion()); metadataBuilder.put(builder); } } + final ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks()); + for (IndexMetadata.APIBlock block : IndexMetadata.APIBlock.values()) { + changed |= maybeUpdateClusterBlock(actualIndices, blocks, block.block, block.setting, openSettings); + } + + if (changed == false) { + return currentState; + } + ClusterState updatedState = ClusterState.builder(currentState).metadata(metadataBuilder) .routingTable(routingTableBuilder.build()).blocks(blocks).build(); @@ -294,18 +300,26 @@ public class MetadataUpdateSettingsService { /** * Updates the cluster block only iff the setting exists in the given settings */ - private static void maybeUpdateClusterBlock(String[] actualIndices, ClusterBlocks.Builder blocks, ClusterBlock block, + private static boolean maybeUpdateClusterBlock(String[] actualIndices, ClusterBlocks.Builder blocks, ClusterBlock block, Setting setting, Settings openSettings) { + boolean changed = false; if (setting.exists(openSettings)) { final boolean updateBlock = setting.get(openSettings); for (String index : actualIndices) { if (updateBlock) { - blocks.addIndexBlock(index, block); + if (blocks.hasIndexBlock(index, block) == false) { + blocks.addIndexBlock(index, block); + changed = true; + } } else { - blocks.removeIndexBlock(index, block); + if (blocks.hasIndexBlock(index, block)) { + blocks.removeIndexBlock(index, block); + changed = true; + } } } } + return changed; }