From 1ce10a89b3dc7daf9348578425216217e6014c7d Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 27 Apr 2018 15:04:21 -0400 Subject: [PATCH] Do not ignore request analysis/similarity on resize Today when a resize operation is performed, we copy the analysis, similarity, and sort settings from the source index. It is possible for the resize request to include additional index settings including analysis, similarity, and sort settings. We reject sort settings when validating the request. However, we silently ignore analysis and similarity settings on the request that are already set on the source index. Since it is possible to change the analysis and similarity settings on an existing index, this should be considered a bug and the sort of leniency that we abhor. This commit addresses this bug by allowing the request analysis/similarity settings to override the existing analysis/similarity settings on the target. --- docs/reference/indices/shrink-index.asciidoc | 4 +- .../metadata/MetaDataCreateIndexService.java | 6 +- .../MetaDataCreateIndexServiceTests.java | 68 +++++++++++++++---- 3 files changed, 61 insertions(+), 17 deletions(-) diff --git a/docs/reference/indices/shrink-index.asciidoc b/docs/reference/indices/shrink-index.asciidoc index 027cf8b924d36..c671696d6ca68 100644 --- a/docs/reference/indices/shrink-index.asciidoc +++ b/docs/reference/indices/shrink-index.asciidoc @@ -119,9 +119,7 @@ POST my_source_index/_shrink/my_target_index segment. -NOTE: Mappings may not be specified in the `_shrink` request, and all -`index.analysis.*` and `index.similarity.*` settings will be overwritten with -the settings from the source index. +NOTE: Mappings may not be specified in the `_shrink` request. [float] === Monitoring the shrink process diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 690cd1fbe5a3f..37c27fb9b7bd7 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -56,6 +56,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.IndexScopedSettings; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentHelper; @@ -694,8 +695,9 @@ static void prepareResizeIndexSettings(ClusterState currentState, Set ma throw new IllegalStateException("unknown resize type is " + type); } - final Predicate sourceSettingsPredicate = (s) -> s.startsWith("index.similarity.") - || s.startsWith("index.analysis.") || s.startsWith("index.sort."); + final Predicate sourceSettingsPredicate = + (s) -> (s.startsWith("index.similarity.") || s.startsWith("index.analysis.") || s.startsWith("index.sort.")) + && indexSettingsBuilder.keys().contains(s) == false; indexSettingsBuilder // now copy all similarity / analysis / sort settings - this overrides all settings from the user unless they // wanna add extra settings diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java index 6074102cde313..28fbfaefe6d06 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java @@ -51,6 +51,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.min; import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.equalTo; public class MetaDataCreateIndexServiceTests extends ESTestCase { @@ -243,7 +244,7 @@ public void testResizeIndexSettings() { .put("index.version.created", version) .put("index.version.upgraded", upgraded) .put("index.version.minimum_compatible", minCompat.luceneVersion.toString()) - .put("index.analysis.analyzer.my_analyzer.tokenizer", "keyword") + .put("index.analysis.analyzer.default.tokenizer", "keyword") .build())).nodes(DiscoveryNodes.builder().add(newNode("node1"))) .build(); AllocationService service = new AllocationService(Settings.builder().build(), new AllocationDeciders(Settings.EMPTY, @@ -257,17 +258,60 @@ public void testResizeIndexSettings() { routingTable.index(indexName).shardsWithState(ShardRoutingState.INITIALIZING)).routingTable(); clusterState = ClusterState.builder(clusterState).routingTable(routingTable).build(); - Settings.Builder builder = Settings.builder(); - builder.put("index.number_of_shards", 1); - MetaDataCreateIndexService.prepareResizeIndexSettings(clusterState, Collections.emptySet(), builder, - clusterState.metaData().index(indexName).getIndex(), "target", ResizeType.SHRINK); - assertEquals("similarity settings must be copied", "BM25", builder.build().get("index.similarity.default.type")); - assertEquals("analysis settings must be copied", - "keyword", builder.build().get("index.analysis.analyzer.my_analyzer.tokenizer")); - assertEquals("node1", builder.build().get("index.routing.allocation.initial_recovery._id")); - assertEquals("1", builder.build().get("index.allocation.max_retries")); - assertEquals(version, builder.build().getAsVersion("index.version.created", null)); - assertEquals(upgraded, builder.build().getAsVersion("index.version.upgraded", null)); + { + final Settings.Builder builder = Settings.builder(); + builder.put("index.number_of_shards", 1); + MetaDataCreateIndexService.prepareResizeIndexSettings( + clusterState, + Collections.emptySet(), + builder, + clusterState.metaData().index(indexName).getIndex(), + "target", + ResizeType.SHRINK); + final Settings settings = builder.build(); + assertThat("similarity settings must be copied", settings.get("index.similarity.default.type"), equalTo("BM25")); + assertThat( + "analysis settings must be copied", settings.get("index.analysis.analyzer.default.tokenizer"), equalTo("keyword")); + assertThat(settings.get("index.routing.allocation.initial_recovery._id"), equalTo("node1")); + assertThat(settings.get("index.allocation.max_retries"), equalTo("1")); + assertThat(settings.getAsVersion("index.version.created", null), equalTo(version)); + assertThat(settings.getAsVersion("index.version.upgraded", null), equalTo(upgraded)); + } + + // analysis settings from the request are not overwritten + { + final Settings.Builder builder = Settings.builder(); + builder.put("index.number_of_shards", 1); + builder.put("index.analysis.analyzer.default.tokenizer", "whitespace"); + MetaDataCreateIndexService.prepareResizeIndexSettings( + clusterState, + Collections.emptySet(), + builder, + clusterState.metaData().index(indexName).getIndex(), + "target", + ResizeType.SHRINK); + final Settings settings = builder.build(); + assertThat( + "analysis settings are not overwritten", + settings.get("index.analysis.analyzer.default.tokenizer"), + equalTo("whitespace")); + } + + // similarity settings from the request are not overwritten + { + final Settings.Builder builder = Settings.builder(); + builder.put("index.number_of_shards", 1); + builder.put("index.similarity.default.type", "DFR"); + MetaDataCreateIndexService.prepareResizeIndexSettings( + clusterState, + Collections.emptySet(), + builder, + clusterState.metaData().index(indexName).getIndex(), + "target", + ResizeType.SHRINK); + final Settings settings = builder.build(); + assertThat("similarity settings are not overwritten", settings.get("index.similarity.default.type"), equalTo("DFR")); + } } private DiscoveryNode newNode(String nodeId) {