Skip to content

Commit

Permalink
Do not ignore request analysis/similarity on resize
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jasontedor committed Apr 27, 2018
1 parent 8401eac commit 1ce10a8
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 17 deletions.
4 changes: 1 addition & 3 deletions docs/reference/indices/shrink-index.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -694,8 +695,9 @@ static void prepareResizeIndexSettings(ClusterState currentState, Set<String> ma
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down

0 comments on commit 1ce10a8

Please sign in to comment.