Browse Source

Do not ignore request analysis/similarity on resize (#30216)

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.
Jason Tedor 7 years ago
parent
commit
811f5b4efc

+ 2 - 0
docs/CHANGELOG.asciidoc

@@ -41,6 +41,8 @@ written to by an older Elasticsearch after writing to it with a newer Elasticsea
 
 === Bug Fixes
 
+Do not ignore request analysis/similarity settings on index resize operations when the source index already contains such settings ({pull}30216[#30216])
+
 === Regressions
 
 === Known Issues

+ 4 - 3
docs/reference/indices/shrink-index.asciidoc

@@ -119,9 +119,10 @@ 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.
+
+NOTE: By default, with the exception of `index.analysis`, `index.similarity`, and `index.sort` settings, index settings on the source
+index are not copied during a shrink operation.
 
 [float]
 === Monitoring the shrink process

+ 4 - 3
docs/reference/indices/split-index.asciidoc

@@ -175,9 +175,10 @@ POST my_source_index/_split/my_target_index
     number of shards in the source index.
 
 
-NOTE: Mappings may not be specified in the `_split` 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 `_split` request.
+
+NOTE: By default, with the exception of `index.analysis`, `index.similarity`, and `index.sort` settings, index settings on the source
+index are not copied during a shrink operation.
 
 [float]
 === Monitoring the split process

+ 4 - 2
server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java

@@ -56,6 +56,7 @@ import org.elasticsearch.common.compress.CompressedXContent;
 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 @@ public class MetaDataCreateIndexService extends AbstractComponent {
             throw new IllegalStateException("unknown resize type is " + type);
         }
 
-        final Predicate<String> sourceSettingsPredicate = (s) -> s.startsWith("index.similarity.")
-            || s.startsWith("index.analysis.") || s.startsWith("index.sort.");
+        final Predicate<String> 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

+ 56 - 12
server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java

@@ -51,6 +51,7 @@ import java.util.List;
 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 class MetaDataCreateIndexServiceTests extends ESTestCase {
                 .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 class MetaDataCreateIndexServiceTests extends ESTestCase {
             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) {