From a110498ad8a5e7cf389ebd21d8d7074f7ec286c2 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 1 Sep 2016 07:44:27 +0200 Subject: [PATCH] settings: Make `action.auto_create_index` setting a dynamic cluster setting. Closes #7513 --- .../resources/checkstyle_suppressions.xml | 2 +- .../elasticsearch/action/ActionModule.java | 3 +- .../action/support/AutoCreateIndex.java | 29 +++++-- .../action/support/AutoCreateIndexTests.java | 85 ++++++++++++++----- .../mapper/DynamicMappingDisabledTests.java | 4 +- .../ReindexSourceTargetValidationTests.java | 6 +- 6 files changed, 97 insertions(+), 32 deletions(-) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index 3578fab995359..513495208101a 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -626,7 +626,7 @@ - + diff --git a/core/src/main/java/org/elasticsearch/action/ActionModule.java b/core/src/main/java/org/elasticsearch/action/ActionModule.java index ca5349661c522..1be1ddda9a470 100644 --- a/core/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/core/src/main/java/org/elasticsearch/action/ActionModule.java @@ -20,7 +20,6 @@ package org.elasticsearch.action; import java.util.ArrayList; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -335,7 +334,7 @@ public ActionModule(boolean ingestEnabled, boolean transportClient, Settings set this.actionPlugins = actionPlugins; actions = setupActions(actionPlugins); actionFilters = setupActionFilters(actionPlugins, ingestEnabled); - autoCreateIndex = transportClient ? null : new AutoCreateIndex(settings, resolver); + autoCreateIndex = transportClient ? null : new AutoCreateIndex(settings, clusterSettings, resolver); destructiveOperations = new DestructiveOperations(settings, clusterSettings); Set headers = actionPlugins.stream().flatMap(p -> p.getRestHeaders().stream()).collect(Collectors.toSet()); restController = new RestController(settings, headers); diff --git a/core/src/main/java/org/elasticsearch/action/support/AutoCreateIndex.java b/core/src/main/java/org/elasticsearch/action/support/AutoCreateIndex.java index d4ddae7822559..5c9152b475179 100644 --- a/core/src/main/java/org/elasticsearch/action/support/AutoCreateIndex.java +++ b/core/src/main/java/org/elasticsearch/action/support/AutoCreateIndex.java @@ -24,8 +24,8 @@ import org.elasticsearch.common.Booleans; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.regex.Regex; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; @@ -41,16 +41,17 @@ public final class AutoCreateIndex { public static final Setting AUTO_CREATE_INDEX_SETTING = - new Setting<>("action.auto_create_index", "true", AutoCreate::new, Property.NodeScope); + new Setting<>("action.auto_create_index", "true", AutoCreate::new, Property.NodeScope, Setting.Property.Dynamic); private final boolean dynamicMappingDisabled; private final IndexNameExpressionResolver resolver; - private final AutoCreate autoCreate; + private volatile AutoCreate autoCreate; - public AutoCreateIndex(Settings settings, IndexNameExpressionResolver resolver) { + public AutoCreateIndex(Settings settings, ClusterSettings clusterSettings, IndexNameExpressionResolver resolver) { this.resolver = resolver; dynamicMappingDisabled = !MapperService.INDEX_MAPPER_DYNAMIC_SETTING.get(settings); this.autoCreate = AUTO_CREATE_INDEX_SETTING.get(settings); + clusterSettings.addSettingsUpdateConsumer(AUTO_CREATE_INDEX_SETTING, this::setAutoCreate); } /** @@ -64,6 +65,8 @@ public boolean needToCheck() { * Should the index be auto created? */ public boolean shouldAutoCreate(String index, ClusterState state) { + // One volatile read, so that all checks are done against the same instance: + final AutoCreate autoCreate = this.autoCreate; if (autoCreate.autoCreateIndex == false) { return false; } @@ -87,7 +90,15 @@ public boolean shouldAutoCreate(String index, ClusterState state) { return false; } - private static class AutoCreate { + AutoCreate getAutoCreate() { + return autoCreate; + } + + void setAutoCreate(AutoCreate autoCreate) { + this.autoCreate = autoCreate; + } + + static class AutoCreate { private final boolean autoCreateIndex; private final List> expressions; @@ -128,5 +139,13 @@ private AutoCreate(String value) { this.expressions = expressions; this.autoCreateIndex = autoCreateIndex; } + + boolean isAutoCreateIndex() { + return autoCreateIndex; + } + + List> getExpressions() { + return expressions; + } } } diff --git a/core/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java b/core/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java index 9572a2df6525e..8a45ca4753565 100644 --- a/core/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java +++ b/core/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.test.ESTestCase; @@ -35,10 +36,12 @@ public class AutoCreateIndexTests extends ESTestCase { public void testParseFailed() { try { - new AutoCreateIndex(Settings.builder().put("action.auto_create_index", ",,,").build(), new IndexNameExpressionResolver(Settings.EMPTY)); + Settings settings = Settings.builder().put("action.auto_create_index", ",,,").build(); + newAutoCreateIndex(settings); fail("initialization should have failed"); } catch (IllegalArgumentException ex) { - assertEquals("Can't parse [,,,] for setting [action.auto_create_index] must be either [true, false, or a comma separated list of index patterns]", ex.getMessage()); + assertEquals("Can't parse [,,,] for setting [action.auto_create_index] must be either [true, false, or a " + + "comma separated list of index patterns]", ex.getMessage()); } } @@ -46,46 +49,51 @@ public void testParseFailedMissingIndex() { String prefix = randomFrom("+", "-"); Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), prefix).build(); try { - new AutoCreateIndex(settings, new IndexNameExpressionResolver(settings)); + newAutoCreateIndex(settings); fail("initialization should have failed"); } catch(IllegalArgumentException ex) { - assertEquals("Can't parse [" + prefix + "] for setting [action.auto_create_index] must contain an index name after [" + prefix + "]", ex.getMessage()); + assertEquals("Can't parse [" + prefix + "] for setting [action.auto_create_index] must contain an index name after [" + + prefix + "]", ex.getMessage()); } } public void testAutoCreationDisabled() { Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), false).build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(Settings.EMPTY)); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); assertThat(autoCreateIndex.shouldAutoCreate(randomAsciiOfLengthBetween(1, 10), buildClusterState()), equalTo(false)); } public void testAutoCreationEnabled() { Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), true).build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(Settings.EMPTY)); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); assertThat(autoCreateIndex.shouldAutoCreate(randomAsciiOfLengthBetween(1, 10), buildClusterState()), equalTo(true)); } public void testDefaultAutoCreation() { - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(Settings.EMPTY, new IndexNameExpressionResolver(Settings.EMPTY)); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(Settings.EMPTY); assertThat(autoCreateIndex.shouldAutoCreate(randomAsciiOfLengthBetween(1, 10), buildClusterState()), equalTo(true)); } public void testExistingIndex() { - Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), randomFrom(true, false, randomAsciiOfLengthBetween(7, 10))).build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(Settings.EMPTY)); - assertThat(autoCreateIndex.shouldAutoCreate(randomFrom("index1", "index2", "index3"), buildClusterState("index1", "index2", "index3")), equalTo(false)); + Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), randomFrom(true, false, + randomAsciiOfLengthBetween(7, 10))).build(); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); + assertThat(autoCreateIndex.shouldAutoCreate(randomFrom("index1", "index2", "index3"), + buildClusterState("index1", "index2", "index3")), equalTo(false)); } public void testDynamicMappingDisabled() { - Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), randomFrom(true, randomAsciiOfLengthBetween(1, 10))) + Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), randomFrom(true, + randomAsciiOfLengthBetween(1, 10))) .put(MapperService.INDEX_MAPPER_DYNAMIC_SETTING.getKey(), false).build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(Settings.EMPTY)); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); assertThat(autoCreateIndex.shouldAutoCreate(randomAsciiOfLengthBetween(1, 10), buildClusterState()), equalTo(false)); } public void testAutoCreationPatternEnabled() { - Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), randomFrom("+index*", "index*")).build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(settings)); + Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), randomFrom("+index*", "index*")) + .build(); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build(); assertThat(autoCreateIndex.shouldAutoCreate("index" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(true)); assertThat(autoCreateIndex.shouldAutoCreate("does_not_match" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false)); @@ -93,7 +101,7 @@ public void testAutoCreationPatternEnabled() { public void testAutoCreationPatternDisabled() { Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), "-index*").build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(settings)); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build(); assertThat(autoCreateIndex.shouldAutoCreate("index" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false)); //default is false when patterns are specified @@ -101,8 +109,9 @@ public void testAutoCreationPatternDisabled() { } public void testAutoCreationMultiplePatternsWithWildcards() { - Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), randomFrom("+test*,-index*", "test*,-index*")).build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(settings)); + Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), + randomFrom("+test*,-index*", "test*,-index*")).build(); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build(); assertThat(autoCreateIndex.shouldAutoCreate("index" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false)); assertThat(autoCreateIndex.shouldAutoCreate("test" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(true)); @@ -111,7 +120,7 @@ public void testAutoCreationMultiplePatternsWithWildcards() { public void testAutoCreationMultiplePatternsNoWildcards() { Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), "+test1,-index1").build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(settings)); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build(); assertThat(autoCreateIndex.shouldAutoCreate("test1", clusterState), equalTo(true)); assertThat(autoCreateIndex.shouldAutoCreate("index" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false)); @@ -121,7 +130,7 @@ public void testAutoCreationMultiplePatternsNoWildcards() { public void testAutoCreationMultipleIndexNames() { Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), "test1,test2").build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(settings)); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build(); assertThat(autoCreateIndex.shouldAutoCreate("test1", clusterState), equalTo(true)); assertThat(autoCreateIndex.shouldAutoCreate("test2", clusterState), equalTo(true)); @@ -129,19 +138,51 @@ public void testAutoCreationMultipleIndexNames() { } public void testAutoCreationConflictingPatternsFirstWins() { - Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), "+test1,-test1,-test2,+test2").build(); - AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, new IndexNameExpressionResolver(settings)); + Settings settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), + "+test1,-test1,-test2,+test2").build(); + AutoCreateIndex autoCreateIndex = newAutoCreateIndex(settings); ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()).build(); assertThat(autoCreateIndex.shouldAutoCreate("test1", clusterState), equalTo(true)); assertThat(autoCreateIndex.shouldAutoCreate("test2", clusterState), equalTo(false)); assertThat(autoCreateIndex.shouldAutoCreate("does_not_match" + randomAsciiOfLengthBetween(1, 5), clusterState), equalTo(false)); } + public void testUpdate() { + boolean value = randomBoolean(); + Settings settings; + if (value && randomBoolean()) { + settings = Settings.EMPTY; + } else { + settings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), value).build(); + } + + ClusterSettings clusterSettings = new ClusterSettings(settings, + ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + AutoCreateIndex autoCreateIndex = new AutoCreateIndex(settings, clusterSettings, new IndexNameExpressionResolver(settings)); + assertThat(autoCreateIndex.getAutoCreate().isAutoCreateIndex(), equalTo(value)); + + Settings newSettings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), !value).build(); + clusterSettings.applySettings(newSettings); + assertThat(autoCreateIndex.getAutoCreate().isAutoCreateIndex(), equalTo(!value)); + + newSettings = Settings.builder().put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), "logs-*").build(); + clusterSettings.applySettings(newSettings); + assertThat(autoCreateIndex.getAutoCreate().isAutoCreateIndex(), equalTo(true)); + assertThat(autoCreateIndex.getAutoCreate().getExpressions().size(), equalTo(1)); + assertThat(autoCreateIndex.getAutoCreate().getExpressions().get(0).v1(), equalTo("logs-*")); + } + private static ClusterState buildClusterState(String... indices) { MetaData.Builder metaData = MetaData.builder(); for (String index : indices) { metaData.put(IndexMetaData.builder(index).settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(1)); } - return ClusterState.builder(org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).build(); + return ClusterState.builder(org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metaData(metaData).build(); + } + + private AutoCreateIndex newAutoCreateIndex(Settings settings) { + return new AutoCreateIndex(settings, new ClusterSettings(settings, + ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), new IndexNameExpressionResolver(settings)); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingDisabledTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingDisabledTests.java index 0cd6f93ba3a9b..111f4b470d432 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingDisabledTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingDisabledTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.IndicesService; @@ -81,7 +82,8 @@ public void setUp() throws Exception { shardStateAction = new ShardStateAction(settings, clusterService, transportService, null, null, THREAD_POOL); actionFilters = new ActionFilters(Collections.emptySet()); indexNameExpressionResolver = new IndexNameExpressionResolver(settings); - autoCreateIndex = new AutoCreateIndex(settings, indexNameExpressionResolver); + autoCreateIndex = new AutoCreateIndex(settings, new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), + indexNameExpressionResolver); } @After diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexSourceTargetValidationTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexSourceTargetValidationTests.java index 1213762155b14..12ed0ed090fa6 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexSourceTargetValidationTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexSourceTargetValidationTests.java @@ -31,10 +31,13 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.reindex.remote.RemoteInfo; import org.elasticsearch.test.ESTestCase; +import java.util.Collections; + import static java.util.Collections.emptyMap; import static org.hamcrest.Matchers.containsString; @@ -54,7 +57,8 @@ public class ReindexSourceTargetValidationTests extends ESTestCase { .put(index("source", "source_multi"), true) .put(index("source2", "source_multi"), true)).build(); private static final IndexNameExpressionResolver INDEX_NAME_EXPRESSION_RESOLVER = new IndexNameExpressionResolver(Settings.EMPTY); - private static final AutoCreateIndex AUTO_CREATE_INDEX = new AutoCreateIndex(Settings.EMPTY, INDEX_NAME_EXPRESSION_RESOLVER); + private static final AutoCreateIndex AUTO_CREATE_INDEX = new AutoCreateIndex(Settings.EMPTY, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), INDEX_NAME_EXPRESSION_RESOLVER); public void testObviousCases() { fails("target", "target");