From 3be98927a03fc7bd13b0792baddf1b7492f8855c Mon Sep 17 00:00:00 2001 From: Ruflin Date: Tue, 21 Jul 2015 13:53:16 +0200 Subject: [PATCH 01/11] Fix cidr mask conversion issue for 0.0.0.0/0 and add tests --- .../bucket/range/ipv4/IPv4RangeBuilder.java | 6 +- .../aggregations/bucket/IPv4RangeTests.java | 70 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IPv4RangeBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IPv4RangeBuilder.java index 037afb3f1f9..86eb7eb2bcb 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IPv4RangeBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IPv4RangeBuilder.java @@ -139,6 +139,10 @@ public class IPv4RangeBuilder extends AbstractRangeBuilder { int mask = (-1) << (32 - Integer.parseInt(parts[4])); + if (Integer.parseInt(parts[4]) == 0) { + mask = 0 << 32; + } + int from = addr & mask; long longFrom = intIpToLongIp(from); if (longFrom == 0) { @@ -148,7 +152,7 @@ public class IPv4RangeBuilder extends AbstractRangeBuilder { int to = from + (~mask); long longTo = intIpToLongIp(to) + 1; // we have to +1 here as the range is non-inclusive on the "to" side if (longTo == InternalIPv4Range.MAX_IP) { - longTo = -1; + longTo = InternalIPv4Range.MAX_IP - 1; } return new long[] { longFrom, longTo }; diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java index 1b72e5bfa63..244df1303ea 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.script.Script; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.range.Range; import org.elasticsearch.search.aggregations.bucket.range.Range.Bucket; +import org.elasticsearch.search.aggregations.bucket.range.ipv4.IPv4RangeBuilder; import org.elasticsearch.search.aggregations.metrics.max.Max; import org.elasticsearch.search.aggregations.metrics.sum.Sum; import org.elasticsearch.test.ElasticsearchIntegrationTest; @@ -869,4 +870,73 @@ public class IPv4RangeTests extends ElasticsearchIntegrationTest { assertThat(buckets.get(0).getToAsString(), equalTo("10.0.0.10")); assertThat(buckets.get(0).getDocCount(), equalTo(0l)); } + + @Test + public void cidr0Mask0() { + SearchResponse response = client().prepareSearch("idx_unmapped") + .addAggregation(ipRange("range") + .field("ip") + .addMaskRange("0.0.0.0/0")) + .execute().actionGet(); + + assertSearchResponse(response); + + + Range range = response.getAggregations().get("range"); + assertThat(range, notNullValue()); + assertThat(range.getName(), equalTo("range")); + List buckets = range.getBuckets(); + assertThat(range.getBuckets().size(), equalTo(1)); + + Range.Bucket bucket = buckets.get(0); + assertThat((String) bucket.getKey(), equalTo("0.0.0.0/0")); + + assertThat(bucket.getFromAsString(), nullValue()); + assertThat(bucket.getToAsString(), equalTo("255.255.255.255")); + } + + @Test + public void cidr0Mask1() { + SearchResponse response = client().prepareSearch("idx_unmapped") + .addAggregation(ipRange("range") + .field("ip") + .addMaskRange("0.0.0.0/1")) + .execute().actionGet(); + + assertSearchResponse(response); + + + Range range = response.getAggregations().get("range"); + assertThat(range, notNullValue()); + assertThat(range.getName(), equalTo("range")); + List buckets = range.getBuckets(); + assertThat(range.getBuckets().size(), equalTo(1)); + + Range.Bucket bucket = buckets.get(0); + assertThat((String) bucket.getKey(), equalTo("0.0.0.0/1")); + assertThat(bucket.getFromAsString(), nullValue()); + assertThat(bucket.getToAsString(), equalTo("128.0.0.0")); + } + + @Test + public void cidr0Mask2() { + SearchResponse response = client().prepareSearch("idx_unmapped") + .addAggregation(ipRange("range") + .field("ip") + .addMaskRange("0.0.0.0/2")) + .execute().actionGet(); + + assertSearchResponse(response); + + + Range range = response.getAggregations().get("range"); + assertThat(range, notNullValue()); + assertThat(range.getName(), equalTo("range")); + List buckets = range.getBuckets(); + assertThat(range.getBuckets().size(), equalTo(1)); + + Range.Bucket bucket = buckets.get(0); + assertThat(bucket.getFromAsString(), nullValue()); + assertThat(bucket.getToAsString(), equalTo("64.0.0.0")); + } } From 8ac105dbb931c1a4e091c3bcd82a198da67ff012 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 24 Jul 2015 13:12:44 -0400 Subject: [PATCH 02/11] Add explicit check that we have reached the end of the settings stream when parsing settings Settings are currently parsed by looping over the tokens until an END_OBJECT token is reached. However, this does not mean that the end of the settings stream was reached. This can occur, for example, when parsing a YAML settings file with inconsistent indentation. Currently in this case, some settings will be silently ignored. This commit forces a check that we have in fact reached the end of the settings stream. Closes #12382 --- .../loader/XContentSettingsLoader.java | 18 ++++++++++++++++++ .../common/xcontent/XContentParser.java | 2 ++ .../xcontent/json/JsonXContentParser.java | 5 +++++ .../support/AbstractXContentParser.java | 3 +++ .../loader/YamlSettingsLoaderTests.java | 16 +++++++++++++++- .../settings/loader/indentation-settings.yml | 10 ++++++++++ ...n-with-explicit-document-start-settings.yml | 11 +++++++++++ 7 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/org/elasticsearch/common/settings/loader/indentation-settings.yml create mode 100644 core/src/test/java/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml diff --git a/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java b/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java index ffbe1669d47..d7dcff75e27 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java +++ b/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java @@ -20,6 +20,7 @@ package org.elasticsearch.common.settings.loader; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -65,6 +66,23 @@ public abstract class XContentSettingsLoader implements SettingsLoader { throw new ElasticsearchParseException("malformed, expected settings to start with 'object', instead was [{}]", token); } serializeObject(settings, sb, path, jp, null); + + // ensure we reached the end of the stream + Exception exception = null; + XContentParser.Token lastToken = null; + try { + while (!jp.isClosed() && (lastToken = jp.nextToken()) == null); + } catch (Exception e) { + exception = e; + } + if (exception != null || lastToken != null) { + throw new ElasticsearchParseException( + "malformed, expected end of settings but encountered additional content starting at columnNumber: [{}], lineNumber: [{}]", + jp.getTokenLocation().columnNumber, + jp.getTokenLocation().lineNumber + ); + } + return settings; } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java index 3901a45a181..a6b4f460f47 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java @@ -250,4 +250,6 @@ public interface XContentParser extends Releasable { * @return last token's location or null if cannot be determined */ XContentLocation getTokenLocation(); + + boolean isClosed(); } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java index 5d3a3f99f4e..787c28324de 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java @@ -248,4 +248,9 @@ public class JsonXContentParser extends AbstractXContentParser { } throw new IllegalStateException("No matching token for json_token [" + token + "]"); } + + @Override + public boolean isClosed() { + return parser.isClosed(); + } } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java index f0b157b486d..039fd2629eb 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java @@ -319,4 +319,7 @@ public abstract class AbstractXContentParser implements XContentParser { } return null; } + + @Override + public abstract boolean isClosed(); } diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java b/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java index a9c77e9b310..0ad737cb701 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java @@ -20,11 +20,11 @@ package org.elasticsearch.common.settings.loader; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.Test; import static org.elasticsearch.common.settings.Settings.settingsBuilder; -import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; /** @@ -49,4 +49,18 @@ public class YamlSettingsLoaderTests extends ElasticsearchTestCase { assertThat(settings.getAsArray("test1.test3")[0], equalTo("test3-1")); assertThat(settings.getAsArray("test1.test3")[1], equalTo("test3-2")); } + + @Test(expected = SettingsException.class) + public void testIndentation() { + settingsBuilder() + .loadFromClasspath("org/elasticsearch/common/settings/loader/indentation-settings.yml") + .build(); + } + + @Test(expected = SettingsException.class) + public void testIndentationWithExplicitDocumentStart() { + settingsBuilder() + .loadFromClasspath("org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml") + .build(); + } } \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-settings.yml b/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-settings.yml new file mode 100644 index 00000000000..cd14c5f35a2 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-settings.yml @@ -0,0 +1,10 @@ + test1: + value1: value1 + test2: + value2: value2 + value3: 2 + test3: + - test3-1 + - test3-2 +test4: + value4: value4 diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml b/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml new file mode 100644 index 00000000000..e02a357d89d --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml @@ -0,0 +1,11 @@ + test1: + value1: value1 + test2: + value2: value2 + value3: 2 + test3: + - test3-1 + - test3-2 +--- +test4: + value4: value4 From c73fff799dccccb58aad0b436b4eeedfe2340c01 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 13 Jul 2015 01:30:40 +0200 Subject: [PATCH 03/11] Cleaned up the data structures used in MetaData for alias and index lookups. Major changes: * Changed MetaData to holds alias and index lookup information into a single TreeMap instead of two separate maps. * Moved the building of the alias / index lookup to the metadata builder. --- .../cluster/metadata/AliasOrIndex.java | 138 ++++++++++ .../metadata/IndexNameExpressionResolver.java | 243 ++++++++---------- .../cluster/metadata/MetaData.java | 204 +++++++-------- .../metadata/MetaDataCreateIndexService.java | 2 +- .../metadata/MetaDataIndexAliasesService.java | 2 +- .../gateway/LocalAllocateDangledIndices.java | 2 +- .../cluster/IndicesClusterStateService.java | 2 +- .../aliases/IndexAliasesTests.java | 3 +- .../benchmark/aliases/AliasesBenchmark.java | 6 +- .../cluster/ClusterStateDiffTests.java | 3 +- .../elasticsearch/cluster/ack/AckTests.java | 3 +- 11 files changed, 361 insertions(+), 247 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/cluster/metadata/AliasOrIndex.java diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/AliasOrIndex.java b/core/src/main/java/org/elasticsearch/cluster/metadata/AliasOrIndex.java new file mode 100644 index 00000000000..e98a046238d --- /dev/null +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/AliasOrIndex.java @@ -0,0 +1,138 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.cluster.metadata; + +import com.google.common.collect.UnmodifiableIterator; +import org.elasticsearch.common.collect.Tuple; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + +/** + * Encapsulates the {@link IndexMetaData} instances of a concrete index or indices an alias is pointing to. + */ +public interface AliasOrIndex { + + /** + * @return whether this an alias or concrete index + */ + boolean isAlias(); + + /** + * @return All {@link IndexMetaData} of all concrete indices this alias is referring to or if this is a concrete index its {@link IndexMetaData} + */ + List getIndices(); + + /** + * Represents an concrete index and encapsulates its {@link IndexMetaData} + */ + class Index implements AliasOrIndex { + + private final IndexMetaData concreteIndex; + + public Index(IndexMetaData indexMetaData) { + this.concreteIndex = indexMetaData; + } + + @Override + public boolean isAlias() { + return false; + } + + @Override + public List getIndices() { + return Collections.singletonList(concreteIndex); + } + + /** + * @return If this is an concrete index, its {@link IndexMetaData} + */ + public IndexMetaData getIndex() { + return concreteIndex; + } + + } + + /** + * Represents an alias and groups all {@link IndexMetaData} instances sharing the same alias name together. + */ + class Alias implements AliasOrIndex { + + private final String aliasName; + private final List referenceIndexMetaDatas; + + public Alias(AliasMetaData aliasMetaData, IndexMetaData indexMetaData) { + this.aliasName = aliasMetaData.getAlias(); + this.referenceIndexMetaDatas = new ArrayList<>(); + this.referenceIndexMetaDatas.add(indexMetaData); + } + + @Override + public boolean isAlias() { + return true; + } + + @Override + public List getIndices() { + return referenceIndexMetaDatas; + } + + /** + * Returns the unique alias metadata per concrete index. + * + * (note that although alias can point to the same concrete indices, each alias reference may have its own routing + * and filters) + */ + public Iterable> getConcreteIndexAndAliasMetaDatas() { + return new Iterable>() { + @Override + public Iterator> iterator() { + return new UnmodifiableIterator>() { + + int index = 0; + + @Override + public boolean hasNext() { + return index < referenceIndexMetaDatas.size(); + } + + @Override + public Tuple next() { + IndexMetaData indexMetaData = referenceIndexMetaDatas.get(index++); + return new Tuple<>(indexMetaData.getIndex(), indexMetaData.getAliases().get(aliasName)); + } + + }; + } + }; + } + + public AliasMetaData getFirstAliasMetaData() { + return referenceIndexMetaDatas.get(0).getAliases().get(aliasName); + } + + void addIndex(IndexMetaData indexMetaData) { + this.referenceIndexMetaDatas.add(indexMetaData); + } + + } +} diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 77c35fdcf50..f9454772f05 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -19,16 +19,14 @@ package org.elasticsearch.cluster.metadata; -import com.carrotsearch.hppc.cursors.ObjectCursor; -import com.carrotsearch.hppc.cursors.ObjectObjectCursor; +import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.index.Index; @@ -38,6 +36,7 @@ import org.elasticsearch.indices.IndexClosedException; import java.util.*; import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.filterEntries; import static com.google.common.collect.Maps.newHashMap; public class IndexNameExpressionResolver { @@ -108,44 +107,40 @@ public class IndexNameExpressionResolver { List concreteIndices = new ArrayList<>(expressions.size()); for (String expression : expressions) { - List indexMetaDatas; - IndexMetaData indexMetaData = metaData.getIndices().get(expression); - if (indexMetaData == null) { - ImmutableOpenMap indexAliasMap = metaData.aliases().get(expression); - if (indexAliasMap == null) { - if (failNoIndices) { - IndexNotFoundException infe = new IndexNotFoundException(expression); - infe.setResources("index_expression", expression); - throw infe; - - } else { - continue; - } + AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(expression); + if (aliasOrIndex == null) { + if (failNoIndices) { + IndexNotFoundException infe = new IndexNotFoundException(expression); + infe.setResources("index_expression", expression); + throw infe; + } else { + continue; } - if (indexAliasMap.size() > 1 && !options.allowAliasesToMultipleIndices()) { - throw new IllegalArgumentException("Alias [" + expression + "] has more than one indices associated with it [" + Arrays.toString(indexAliasMap.keys().toArray(String.class)) + "], can't execute a single index op"); - } - indexMetaDatas = new ArrayList<>(indexAliasMap.size()); - for (ObjectObjectCursor cursor : indexAliasMap) { - indexMetaDatas.add(metaData.getIndices().get(cursor.key)); - } - } else { - indexMetaDatas = Collections.singletonList(indexMetaData); } - for (IndexMetaData found : indexMetaDatas) { - if (found.getState() == IndexMetaData.State.CLOSE) { + Collection resolvedIndices = aliasOrIndex.getIndices(); + if (resolvedIndices.size() > 1 && !options.allowAliasesToMultipleIndices()) { + String[] indexNames = new String[resolvedIndices.size()]; + int i = 0; + for (IndexMetaData indexMetaData : resolvedIndices) { + indexNames[i++] = indexMetaData.getIndex(); + } + throw new IllegalArgumentException("Alias [" + expression + "] has more than one indices associated with it [" + Arrays.toString(indexNames) + "], can't execute a single index op"); + } + + for (IndexMetaData index : resolvedIndices) { + if (index.getState() == IndexMetaData.State.CLOSE) { if (failClosed) { - throw new IndexClosedException(new Index(found.getIndex())); + throw new IndexClosedException(new Index(index.getIndex())); } else { if (options.forbidClosedIndices() == false) { - concreteIndices.add(found.getIndex()); + concreteIndices.add(index.getIndex()); } } - } else if (found.getState() == IndexMetaData.State.OPEN) { - concreteIndices.add(found.getIndex()); + } else if (index.getState() == IndexMetaData.State.OPEN) { + concreteIndices.add(index.getIndex()); } else { - throw new IllegalStateException("index state [" + found.getState() + "] not supported"); + throw new IllegalStateException("index state [" + index.getState() + "] not supported"); } } } @@ -264,10 +259,6 @@ public class IndexNameExpressionResolver { return resolveSearchRoutingAllIndices(state.metaData(), routing); } - if (resolvedExpressions.size() == 1) { - return resolveSearchRoutingSingleValue(state.metaData(), routing, resolvedExpressions.get(0)); - } - Map> routings = null; Set paramRouting = null; // List of indices that don't require any routing @@ -277,40 +268,43 @@ public class IndexNameExpressionResolver { } for (String expression : resolvedExpressions) { - ImmutableOpenMap indexToRoutingMap = state.metaData().getAliases().get(expression); - if (indexToRoutingMap != null && !indexToRoutingMap.isEmpty()) { - for (ObjectObjectCursor indexRouting : indexToRoutingMap) { - if (!norouting.contains(indexRouting.key)) { - if (!indexRouting.value.searchRoutingValues().isEmpty()) { + AliasOrIndex aliasOrIndex = state.metaData().getAliasAndIndexLookup().get(expression); + if (aliasOrIndex != null && aliasOrIndex.isAlias()) { + AliasOrIndex.Alias alias = (AliasOrIndex.Alias) aliasOrIndex; + for (Tuple item : alias.getConcreteIndexAndAliasMetaDatas()) { + String concreteIndex = item.v1(); + AliasMetaData aliasMetaData = item.v2(); + if (!norouting.contains(concreteIndex)) { + if (!aliasMetaData.searchRoutingValues().isEmpty()) { // Routing alias if (routings == null) { routings = newHashMap(); } - Set r = routings.get(indexRouting.key); + Set r = routings.get(concreteIndex); if (r == null) { r = new HashSet<>(); - routings.put(indexRouting.key, r); + routings.put(concreteIndex, r); } - r.addAll(indexRouting.value.searchRoutingValues()); + r.addAll(aliasMetaData.searchRoutingValues()); if (paramRouting != null) { r.retainAll(paramRouting); } if (r.isEmpty()) { - routings.remove(indexRouting.key); + routings.remove(concreteIndex); } } else { // Non-routing alias - if (!norouting.contains(indexRouting.key)) { - norouting.add(indexRouting.key); + if (!norouting.contains(concreteIndex)) { + norouting.add(concreteIndex); if (paramRouting != null) { Set r = new HashSet<>(paramRouting); if (routings == null) { routings = newHashMap(); } - routings.put(indexRouting.key, r); + routings.put(concreteIndex, r); } else { if (routings != null) { - routings.remove(indexRouting.key); + routings.remove(concreteIndex); } } } @@ -342,49 +336,6 @@ public class IndexNameExpressionResolver { return routings; } - private Map> resolveSearchRoutingSingleValue(MetaData metaData, @Nullable String routing, String aliasOrIndex) { - Map> routings = null; - Set paramRouting = null; - if (routing != null) { - paramRouting = Strings.splitStringByCommaToSet(routing); - } - - ImmutableOpenMap indexToRoutingMap = metaData.getAliases().get(aliasOrIndex); - if (indexToRoutingMap != null && !indexToRoutingMap.isEmpty()) { - // It's an alias - for (ObjectObjectCursor indexRouting : indexToRoutingMap) { - if (!indexRouting.value.searchRoutingValues().isEmpty()) { - // Routing alias - Set r = new HashSet<>(indexRouting.value.searchRoutingValues()); - if (paramRouting != null) { - r.retainAll(paramRouting); - } - if (!r.isEmpty()) { - if (routings == null) { - routings = newHashMap(); - } - routings.put(indexRouting.key, r); - } - } else { - // Non-routing alias - if (paramRouting != null) { - Set r = new HashSet<>(paramRouting); - if (routings == null) { - routings = newHashMap(); - } - routings.put(indexRouting.key, r); - } - } - } - } else { - // It's an index - if (paramRouting != null) { - routings = ImmutableMap.of(aliasOrIndex, paramRouting); - } - } - return routings; - } - /** * Sets the same routing for all indices */ @@ -494,7 +445,7 @@ public class IndexNameExpressionResolver { return expressions; } - if (expressions.isEmpty() || (expressions.size() == 1 && MetaData.ALL.equals(expressions.get(0)))) { + if (expressions.isEmpty() || (expressions.size() == 1 && (MetaData.ALL.equals(expressions.get(0))) || Regex.isMatchAllPattern(expressions.get(0)))) { if (options.expandWildcardsOpen() && options.expandWildcardsClosed()) { return Arrays.asList(metaData.concreteAllIndices()); } else if (options.expandWildcardsOpen()) { @@ -508,22 +459,22 @@ public class IndexNameExpressionResolver { Set result = null; for (int i = 0; i < expressions.size(); i++) { - String aliasOrIndex = expressions.get(i); - if (metaData.getAliasAndIndexMap().containsKey(aliasOrIndex)) { + String expression = expressions.get(i); + if (metaData.getAliasAndIndexLookup().containsKey(expression)) { if (result != null) { - result.add(aliasOrIndex); + result.add(expression); } continue; } boolean add = true; - if (aliasOrIndex.charAt(0) == '+') { + if (expression.charAt(0) == '+') { // if its the first, add empty result set if (i == 0) { result = new HashSet<>(); } add = true; - aliasOrIndex = aliasOrIndex.substring(1); - } else if (aliasOrIndex.charAt(0) == '-') { + expression = expression.substring(1); + } else if (expression.charAt(0) == '-') { // if its the first, fill it with all the indices... if (i == 0) { String[] concreteIndices; @@ -540,19 +491,19 @@ public class IndexNameExpressionResolver { result = new HashSet<>(Arrays.asList(concreteIndices)); } add = false; - aliasOrIndex = aliasOrIndex.substring(1); + expression = expression.substring(1); } - if (!Regex.isSimpleMatchPattern(aliasOrIndex)) { - if (!options.ignoreUnavailable() && !metaData.getAliasAndIndexMap().containsKey(aliasOrIndex)) { - IndexNotFoundException infe = new IndexNotFoundException(aliasOrIndex); - infe.setResources("index_or_alias", aliasOrIndex); + if (!Regex.isSimpleMatchPattern(expression)) { + if (!options.ignoreUnavailable() && !metaData.getAliasAndIndexLookup().containsKey(expression)) { + IndexNotFoundException infe = new IndexNotFoundException(expression); + infe.setResources("index_or_alias", expression); throw infe; } if (result != null) { if (add) { - result.add(aliasOrIndex); + result.add(expression); } else { - result.remove(aliasOrIndex); + result.remove(expression); } } continue; @@ -562,44 +513,60 @@ public class IndexNameExpressionResolver { result = new HashSet<>(); result.addAll(expressions.subList(0, i)); } - String[] indices; - if (options.expandWildcardsOpen() && options.expandWildcardsClosed()) { - indices = metaData.concreteAllIndices(); - } else if (options.expandWildcardsOpen()) { - indices = metaData.concreteAllOpenIndices(); - } else if (options.expandWildcardsClosed()) { - indices = metaData.concreteAllClosedIndices(); + + final IndexMetaData.State excludeState; + if (options.expandWildcardsOpen() && options.expandWildcardsClosed()){ + excludeState = null; + } else if (options.expandWildcardsOpen() && options.expandWildcardsClosed() == false) { + excludeState = IndexMetaData.State.CLOSE; + } else if (options.expandWildcardsClosed() && options.expandWildcardsOpen() == false) { + excludeState = IndexMetaData.State.OPEN; } else { assert false : "this shouldn't get called if wildcards expand to none"; - indices = Strings.EMPTY_ARRAY; + excludeState = null; } - boolean found = false; - // iterating over all concrete indices and see if there is a wildcard match - for (String index : indices) { - if (Regex.simpleMatch(aliasOrIndex, index)) { - found = true; - if (add) { - result.add(index); - } else { - result.remove(index); + + final Map matches; + if (Regex.isMatchAllPattern(expression)) { + // Can only happen if the expressions was initially: '-*' + matches = metaData.getAliasAndIndexLookup(); + } else if (expression.endsWith("*")) { + // Suffix wildcard: + assert expression.length() >= 2 : "expression [" + expression + "] should have at least a length of 2"; + String fromPrefix = expression.substring(0, expression.length() - 1); + char[] toPrefixCharArr = fromPrefix.toCharArray(); + toPrefixCharArr[toPrefixCharArr.length - 1]++; + String toPrefix = new String(toPrefixCharArr); + matches = metaData.getAliasAndIndexLookup().subMap(fromPrefix, toPrefix); + } else { + // Other wildcard expressions: + final String pattern = expression; + matches = filterEntries(metaData.getAliasAndIndexLookup(), new Predicate>() { + @Override + public boolean apply(@Nullable Map.Entry input) { + return Regex.simpleMatch(pattern, input.getKey()); + } + }); + } + for (Map.Entry entry : matches.entrySet()) { + AliasOrIndex aliasOrIndex = entry.getValue(); + if (aliasOrIndex.isAlias() == false) { + AliasOrIndex.Index index = (AliasOrIndex.Index) aliasOrIndex; + if (excludeState != null && index.getIndex().getState() == excludeState) { + continue; } } - } - // iterating over all aliases and see if there is a wildcard match - for (ObjectCursor cursor : metaData.getAliases().keys()) { - String alias = cursor.value; - if (Regex.simpleMatch(aliasOrIndex, alias)) { - found = true; - if (add) { - result.add(alias); - } else { - result.remove(alias); - } + + if (add) { + result.add(entry.getKey()); + } else { + result.remove(entry.getKey()); } } - if (!found && !options.allowNoIndices()) { - IndexNotFoundException infe = new IndexNotFoundException(aliasOrIndex); - infe.setResources("index_or_alias", aliasOrIndex); + + if (matches.isEmpty() && options.allowNoIndices() == false) { + IndexNotFoundException infe = new IndexNotFoundException(expression); + infe.setResources("index_or_alias", expression); throw infe; } } diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 3715e64f7b1..32098e58c83 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -19,7 +19,6 @@ package org.elasticsearch.cluster.metadata; -import com.carrotsearch.hppc.ObjectArrayList; import com.carrotsearch.hppc.ObjectHashSet; import com.carrotsearch.hppc.cursors.ObjectCursor; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; @@ -48,7 +47,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.loader.SettingsLoader; import org.elasticsearch.common.xcontent.*; import org.elasticsearch.discovery.DiscoverySettings; -import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.recovery.RecoverySettings; import org.elasticsearch.indices.store.IndicesStore; @@ -146,16 +144,14 @@ public class MetaData implements Iterable, Diffable, Fr private final transient int totalNumberOfShards; // Transient ? not serializable anyway? private final int numberOfShards; - private final String[] allIndices; private final String[] allOpenIndices; private final String[] allClosedIndices; - private final ImmutableOpenMap> aliases; - private final ImmutableOpenMap aliasAndIndexToIndexMap; + private final SortedMap aliasAndIndexLookup; @SuppressWarnings("unchecked") - MetaData(String clusterUUID, long version, Settings transientSettings, Settings persistentSettings, ImmutableOpenMap indices, ImmutableOpenMap templates, ImmutableOpenMap customs) { + MetaData(String clusterUUID, long version, Settings transientSettings, Settings persistentSettings, ImmutableOpenMap indices, ImmutableOpenMap templates, ImmutableOpenMap customs, String[] allIndices, String[] allOpenIndices, String[] allClosedIndices, SortedMap aliasAndIndexLookup) { this.clusterUUID = clusterUUID; this.version = version; this.transientSettings = transientSettings; @@ -166,87 +162,17 @@ public class MetaData implements Iterable, Diffable, Fr this.templates = templates; int totalNumberOfShards = 0; int numberOfShards = 0; - int numAliases = 0; for (ObjectCursor cursor : indices.values()) { totalNumberOfShards += cursor.value.totalNumberOfShards(); numberOfShards += cursor.value.numberOfShards(); - numAliases += cursor.value.aliases().size(); } this.totalNumberOfShards = totalNumberOfShards; this.numberOfShards = numberOfShards; - // build all indices map - List allIndicesLst = Lists.newArrayList(); - for (ObjectCursor cursor : indices.values()) { - allIndicesLst.add(cursor.value.index()); - } - allIndices = allIndicesLst.toArray(new String[allIndicesLst.size()]); - int numIndices = allIndicesLst.size(); - - List allOpenIndices = Lists.newArrayList(); - List allClosedIndices = Lists.newArrayList(); - for (ObjectCursor cursor : indices.values()) { - IndexMetaData indexMetaData = cursor.value; - if (indexMetaData.state() == IndexMetaData.State.OPEN) { - allOpenIndices.add(indexMetaData.index()); - } else if (indexMetaData.state() == IndexMetaData.State.CLOSE) { - allClosedIndices.add(indexMetaData.index()); - } - } - this.allOpenIndices = allOpenIndices.toArray(new String[allOpenIndices.size()]); - this.allClosedIndices = allClosedIndices.toArray(new String[allClosedIndices.size()]); - - // build aliases map - ImmutableOpenMap.Builder tmpAliases = ImmutableOpenMap.builder(numAliases); - for (ObjectCursor cursor : indices.values()) { - IndexMetaData indexMetaData = cursor.value; - String index = indexMetaData.index(); - for (ObjectCursor aliasCursor : indexMetaData.aliases().values()) { - AliasMetaData aliasMd = aliasCursor.value; - ImmutableOpenMap.Builder indexAliasMap = (ImmutableOpenMap.Builder) tmpAliases.get(aliasMd.alias()); - if (indexAliasMap == null) { - indexAliasMap = ImmutableOpenMap.builder(1); // typically, there is 1 alias pointing to an index - tmpAliases.put(aliasMd.alias(), indexAliasMap); - } - indexAliasMap.put(index, aliasMd); - } - } - - for (ObjectCursor cursor : tmpAliases.keys()) { - String alias = cursor.value; - // if there is access to the raw values buffer of the map that the immutable maps wraps, then we don't need to use put, and just set array slots - ImmutableOpenMap map = ((ImmutableOpenMap.Builder) tmpAliases.get(alias)).cast().build(); - tmpAliases.put(alias, map); - } - - this.aliases = tmpAliases.>cast().build(); - ImmutableOpenMap.Builder aliasAndIndexToIndexMap = ImmutableOpenMap.builder(numAliases + numIndices); - for (ObjectCursor cursor : indices.values()) { - IndexMetaData indexMetaData = cursor.value; - ObjectArrayList indicesLst = (ObjectArrayList) aliasAndIndexToIndexMap.get(indexMetaData.index()); - if (indicesLst == null) { - indicesLst = new ObjectArrayList<>(); - aliasAndIndexToIndexMap.put(indexMetaData.index(), indicesLst); - } - indicesLst.add(indexMetaData.index()); - - for (ObjectCursor cursor1 : indexMetaData.aliases().keys()) { - String alias = cursor1.value; - indicesLst = (ObjectArrayList) aliasAndIndexToIndexMap.get(alias); - if (indicesLst == null) { - indicesLst = new ObjectArrayList<>(); - aliasAndIndexToIndexMap.put(alias, indicesLst); - } - indicesLst.add(indexMetaData.index()); - } - } - - for (ObjectObjectCursor cursor : aliasAndIndexToIndexMap) { - String[] indicesLst = ((ObjectArrayList) cursor.value).toArray(String.class); - aliasAndIndexToIndexMap.put(cursor.key, indicesLst); - } - - this.aliasAndIndexToIndexMap = aliasAndIndexToIndexMap.cast().build(); + this.allIndices = allIndices; + this.allOpenIndices = allOpenIndices; + this.allClosedIndices = allClosedIndices; + this.aliasAndIndexLookup = aliasAndIndexLookup; } public long version() { @@ -272,16 +198,32 @@ public class MetaData implements Iterable, Diffable, Fr return this.persistentSettings; } - public ImmutableOpenMap> aliases() { - return this.aliases; + public boolean hasAlias(String alias) { + AliasOrIndex aliasOrIndex = getAliasAndIndexLookup().get(alias); + if (aliasOrIndex != null) { + return aliasOrIndex.isAlias(); + } else { + return false; + } } - public ImmutableOpenMap> getAliases() { - return aliases(); + public boolean equalsAliases(MetaData other) { + for (ObjectCursor cursor : other.indices().values()) { + IndexMetaData otherIndex = cursor.value; + IndexMetaData thisIndex= indices().get(otherIndex.getIndex()); + if (thisIndex == null) { + return false; + } + if (otherIndex.getAliases().equals(thisIndex.getAliases()) == false) { + return false; + } + } + + return true; } - public ImmutableOpenMap getAliasAndIndexMap() { - return aliasAndIndexToIndexMap; + public SortedMap getAliasAndIndexLookup() { + return aliasAndIndexLookup; } /** @@ -477,15 +419,24 @@ public class MetaData implements Iterable, Diffable, Fr // TODO: This can be moved to IndexNameExpressionResolver too, but this means that we will support wildcards and other expressions // in the index,bulk,update and delete apis. public String resolveIndexRouting(@Nullable String routing, String aliasOrIndex) { - // Check if index is specified by an alias - ImmutableOpenMap indexAliases = aliases.get(aliasOrIndex); - if (indexAliases == null || indexAliases.isEmpty()) { + if (aliasOrIndex == null) { return routing; } - if (indexAliases.size() > 1) { - throw new IllegalArgumentException("Alias [" + aliasOrIndex + "] has more than one index associated with it [" + Arrays.toString(indexAliases.keys().toArray(String.class)) + "], can't execute a single index op"); + + AliasOrIndex result = getAliasAndIndexLookup().get(aliasOrIndex); + if (result == null || result.isAlias() == false) { + return routing; } - AliasMetaData aliasMd = indexAliases.values().iterator().next().value; + AliasOrIndex.Alias alias = (AliasOrIndex.Alias) result; + if (result.getIndices().size() > 1) { + String[] indexNames = new String[result.getIndices().size()]; + int i = 0; + for (IndexMetaData indexMetaData : result.getIndices()) { + indexNames[i++] = indexMetaData.getIndex(); + } + throw new IllegalArgumentException("Alias [" + aliasOrIndex + "] has more than one index associated with it [" + Arrays.toString(indexNames) + "], can't execute a single index op"); + } + AliasMetaData aliasMd = alias.getFirstAliasMetaData(); if (aliasMd.indexRouting() != null) { if (routing != null) { if (!routing.equals(aliasMd.indexRouting())) { @@ -507,7 +458,7 @@ public class MetaData implements Iterable, Diffable, Fr } public boolean hasConcreteIndex(String index) { - return aliasAndIndexToIndexMap.containsKey(index); + return getAliasAndIndexLookup().containsKey(index); } public IndexMetaData index(String index) { @@ -829,13 +780,18 @@ public class MetaData implements Iterable, Diffable, Fr } if (newPersistentSettings != null) { - return new MetaData(metaData.clusterUUID(), - metaData.version(), - metaData.transientSettings(), - newPersistentSettings.build(), - metaData.getIndices(), - metaData.getTemplates(), - metaData.getCustoms()); + return new MetaData( + metaData.clusterUUID(), + metaData.version(), + metaData.transientSettings(), + newPersistentSettings.build(), + metaData.getIndices(), + metaData.getTemplates(), + metaData.getCustoms(), + metaData.concreteAllIndices(), + metaData.concreteAllOpenIndices(), + metaData.concreteAllClosedIndices(), + metaData.getAliasAndIndexLookup()); } else { // No changes: return metaData; @@ -1013,7 +969,53 @@ public class MetaData implements Iterable, Diffable, Fr } public MetaData build() { - return new MetaData(clusterUUID, version, transientSettings, persistentSettings, indices.build(), templates.build(), customs.build()); + // TODO: We should move these datastructures to IndexNameExpressionResolver, this will give the following benefits: + // 1) The datastructures will only be rebuilded when needed. Now during serailizing we rebuild these datastructures + // while these datastructures aren't even used. + // 2) The aliasAndIndexLookup can be updated instead of rebuilding it all the time. + + // build all concrete indices arrays: + // TODO: I think we can remove these arrays. it isn't worth the effort, for operations on all indices. + // When doing an operation across all indices, most of the time is spent on actually going to all shards and + // do the required operations, the bottleneck isn't resolving expressions into concrete indices. + List allIndicesLst = Lists.newArrayList(); + for (ObjectCursor cursor : indices.values()) { + allIndicesLst.add(cursor.value.index()); + } + String[] allIndices = allIndicesLst.toArray(new String[allIndicesLst.size()]); + + List allOpenIndicesLst = Lists.newArrayList(); + List allClosedIndicesLst = Lists.newArrayList(); + for (ObjectCursor cursor : indices.values()) { + IndexMetaData indexMetaData = cursor.value; + if (indexMetaData.state() == IndexMetaData.State.OPEN) { + allOpenIndicesLst.add(indexMetaData.index()); + } else if (indexMetaData.state() == IndexMetaData.State.CLOSE) { + allClosedIndicesLst.add(indexMetaData.index()); + } + } + String[] allOpenIndices = allOpenIndicesLst.toArray(new String[allOpenIndicesLst.size()]); + String[] allClosedIndices = allClosedIndicesLst.toArray(new String[allClosedIndicesLst.size()]); + + // build all indices map + SortedMap aliasAndIndexLookup = new TreeMap<>(); + for (ObjectCursor cursor : indices.values()) { + IndexMetaData indexMetaData = cursor.value; + aliasAndIndexLookup.put(indexMetaData.getIndex(), new AliasOrIndex.Index(indexMetaData)); + + for (ObjectObjectCursor aliasCursor : indexMetaData.getAliases()) { + AliasMetaData aliasMetaData = aliasCursor.value; + AliasOrIndex.Alias aliasOrIndex = (AliasOrIndex.Alias) aliasAndIndexLookup.get(aliasMetaData.getAlias()); + if (aliasOrIndex == null) { + aliasOrIndex = new AliasOrIndex.Alias(aliasMetaData, indexMetaData); + aliasAndIndexLookup.put(aliasMetaData.getAlias(), aliasOrIndex); + } else { + aliasOrIndex.addIndex(indexMetaData); + } + } + } + aliasAndIndexLookup = Collections.unmodifiableSortedMap(aliasAndIndexLookup); + return new MetaData(clusterUUID, version, transientSettings, persistentSettings, indices.build(), templates.build(), customs.build(), allIndices, allOpenIndices, allClosedIndices, aliasAndIndexLookup); } public static String toXContent(MetaData metaData) throws IOException { diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index c3773cec62c..7f70886116d 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -181,7 +181,7 @@ public class MetaDataCreateIndexService extends AbstractComponent { "index name is too long, (" + byteCount + " > " + MAX_INDEX_NAME_BYTES + ")"); } - if (state.metaData().aliases().containsKey(index)) { + if (state.metaData().hasAlias(index)) { throw new InvalidIndexNameException(new Index(index), index, "already exists as alias"); } } diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexAliasesService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexAliasesService.java index fb2e933dcfb..d5512f63172 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexAliasesService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexAliasesService.java @@ -145,7 +145,7 @@ public class MetaDataIndexAliasesService extends AbstractComponent { ClusterState updatedState = ClusterState.builder(currentState).metaData(builder).build(); // even though changes happened, they resulted in 0 actual changes to metadata // i.e. remove and add the same alias to the same index - if (!updatedState.metaData().aliases().equals(currentState.metaData().aliases())) { + if (!updatedState.metaData().equalsAliases(currentState.metaData())) { return updatedState; } } diff --git a/core/src/main/java/org/elasticsearch/gateway/LocalAllocateDangledIndices.java b/core/src/main/java/org/elasticsearch/gateway/LocalAllocateDangledIndices.java index 80ecee29eb7..36c3af0d60b 100644 --- a/core/src/main/java/org/elasticsearch/gateway/LocalAllocateDangledIndices.java +++ b/core/src/main/java/org/elasticsearch/gateway/LocalAllocateDangledIndices.java @@ -128,7 +128,7 @@ public class LocalAllocateDangledIndices extends AbstractComponent { if (currentState.metaData().hasIndex(indexMetaData.index())) { continue; } - if (currentState.metaData().aliases().containsKey(indexMetaData.index())) { + if (currentState.metaData().hasAlias(indexMetaData.index())) { logger.warn("ignoring dangled index [{}] on node [{}] due to an existing alias with the same name", indexMetaData.index(), request.fromNode); continue; diff --git a/core/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java b/core/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java index 9dd4aabcf0d..fd94546acde 100644 --- a/core/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java +++ b/core/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java @@ -438,7 +438,7 @@ public class IndicesClusterStateService extends AbstractLifecycleComponent verify that filter was updated"); - AliasMetaData aliasMetaData = internalCluster().clusterService().state().metaData().aliases().get("alias1").get("test"); + AliasMetaData aliasMetaData = ((AliasOrIndex.Alias) internalCluster().clusterService().state().metaData().getAliasAndIndexLookup().get("alias1")).getFirstAliasMetaData(); assertThat(aliasMetaData.getFilter().toString(), equalTo("{\"term\":{\"name\":\"bar\"}}")); logger.info("--> deleting alias1"); diff --git a/core/src/test/java/org/elasticsearch/benchmark/aliases/AliasesBenchmark.java b/core/src/test/java/org/elasticsearch/benchmark/aliases/AliasesBenchmark.java index a225024b1c2..7b5d489e45f 100644 --- a/core/src/test/java/org/elasticsearch/benchmark/aliases/AliasesBenchmark.java +++ b/core/src/test/java/org/elasticsearch/benchmark/aliases/AliasesBenchmark.java @@ -26,6 +26,7 @@ import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.indices.IndexAlreadyExistsException; +import org.elasticsearch.monitor.jvm.JvmStats; import org.elasticsearch.node.Node; import org.elasticsearch.node.NodeBuilder; @@ -39,7 +40,7 @@ public class AliasesBenchmark { private final static String INDEX_NAME = "my-index"; public static void main(String[] args) throws IOException { - int NUM_ADDITIONAL_NODES = 0; + int NUM_ADDITIONAL_NODES = 1; int BASE_ALIAS_COUNT = 100000; int NUM_ADD_ALIAS_REQUEST = 1000; @@ -104,6 +105,7 @@ public class AliasesBenchmark { if (i != numberOfAliases && i % 100 == 0) { long avgTime = totalTime / 100; System.out.println("Added [" + (i - numberOfAliases) + "] aliases. Avg create time: " + avgTime + " ms"); + System.out.println("Heap used [" + JvmStats.jvmStats().getMem().getHeapUsed() + "]"); totalTime = 0; } @@ -113,6 +115,8 @@ public class AliasesBenchmark { .execute().actionGet(); totalTime += System.currentTimeMillis() - time; } + System.gc(); + System.out.println("Final heap used [" + JvmStats.jvmStats().getMem().getHeapUsed() + "]"); System.out.println("Number of aliases: " + countAliases(client)); client.close(); diff --git a/core/src/test/java/org/elasticsearch/cluster/ClusterStateDiffTests.java b/core/src/test/java/org/elasticsearch/cluster/ClusterStateDiffTests.java index 60f3c35c854..be805757174 100644 --- a/core/src/test/java/org/elasticsearch/cluster/ClusterStateDiffTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/ClusterStateDiffTests.java @@ -51,6 +51,7 @@ import static org.elasticsearch.test.XContentTestUtils.convertToMap; import static org.elasticsearch.test.XContentTestUtils.mapsEqualIgnoringArrayOrder; import static org.elasticsearch.test.VersionUtils.randomVersion; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; @ElasticsearchIntegrationTest.ClusterScope(scope = ElasticsearchIntegrationTest.Scope.SUITE, numDataNodes = 0, numClientNodes = 0) @@ -147,7 +148,7 @@ public class ClusterStateDiffTests extends ElasticsearchIntegrationTest { assertThat(clusterStateFromDiffs.metaData().indices(), equalTo(clusterState.metaData().indices())); assertThat(clusterStateFromDiffs.metaData().templates(), equalTo(clusterState.metaData().templates())); assertThat(clusterStateFromDiffs.metaData().customs(), equalTo(clusterState.metaData().customs())); - assertThat(clusterStateFromDiffs.metaData().aliases(), equalTo(clusterState.metaData().aliases())); + assertThat(clusterStateFromDiffs.metaData().equalsAliases(clusterState.metaData()), is(true)); // JSON Serialization test - make sure that both states produce similar JSON assertThat(mapsEqualIgnoringArrayOrder(convertToMap(clusterStateFromDiffs), convertToMap(clusterState)), equalTo(true)); diff --git a/core/src/test/java/org/elasticsearch/cluster/ack/AckTests.java b/core/src/test/java/org/elasticsearch/cluster/ack/AckTests.java index 08445ff84d0..f287a95acab 100644 --- a/core/src/test/java/org/elasticsearch/cluster/ack/AckTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/ack/AckTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.action.admin.indices.warmer.put.PutWarmerResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.AliasMetaData; +import org.elasticsearch.cluster.metadata.AliasOrIndex; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.RoutingNode; @@ -310,7 +311,7 @@ public class AckTests extends ElasticsearchIntegrationTest { assertAcked(client().admin().indices().prepareAliases().addAlias("test", "alias")); for (Client client : clients()) { - AliasMetaData aliasMetaData = getLocalClusterState(client).metaData().aliases().get("alias").get("test"); + AliasMetaData aliasMetaData = ((AliasOrIndex.Alias) getLocalClusterState(client).metaData().getAliasAndIndexLookup().get("alias")).getFirstAliasMetaData(); assertThat(aliasMetaData.alias(), equalTo("alias")); } } From b3272fe6483ce661d6f6284b967174b767d01ba7 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 25 Jul 2015 15:29:15 -0400 Subject: [PATCH 04/11] Preserve the root cause when encountering an exception expecting to have reached the end of the setting stream --- .../loader/XContentSettingsLoader.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java b/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java index d7dcff75e27..e3e08fb93f2 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java +++ b/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java @@ -68,18 +68,22 @@ public abstract class XContentSettingsLoader implements SettingsLoader { serializeObject(settings, sb, path, jp, null); // ensure we reached the end of the stream - Exception exception = null; XContentParser.Token lastToken = null; try { while (!jp.isClosed() && (lastToken = jp.nextToken()) == null); } catch (Exception e) { - exception = e; - } - if (exception != null || lastToken != null) { throw new ElasticsearchParseException( - "malformed, expected end of settings but encountered additional content starting at columnNumber: [{}], lineNumber: [{}]", - jp.getTokenLocation().columnNumber, - jp.getTokenLocation().lineNumber + "malformed, expected end of settings but encountered additional content starting at line number: [{}], column number: [{}]", + e, + jp.getTokenLocation().lineNumber, + jp.getTokenLocation().columnNumber + ); + } + if (lastToken != null) { + throw new ElasticsearchParseException( + "malformed, expected end of settings but encountered additional content starting at line number: [{}], column number: [{}]", + jp.getTokenLocation().lineNumber, + jp.getTokenLocation().columnNumber ); } From b0b9c121c8cb58272e5db894e10431de0ee45895 Mon Sep 17 00:00:00 2001 From: xuzha Date: Sun, 26 Jul 2015 01:16:29 -0700 Subject: [PATCH 05/11] skip hidden files --- .../main/java/org/elasticsearch/plugins/PluginsService.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java index 158f6400bee..abb313c0a82 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -312,6 +312,10 @@ public class PluginsService extends AbstractComponent { try (DirectoryStream stream = Files.newDirectoryStream(pluginsDirectory)) { for (Path plugin : stream) { try { + if (Files.isHidden(plugin)) { + logger.trace("--- skip hidden plugin file[{}]", plugin.toAbsolutePath()); + continue; + } logger.trace("--- adding plugin [{}]", plugin.toAbsolutePath()); PluginInfo info = PluginInfo.readFromProperties(plugin); List urls = new ArrayList<>(); From a99ccb6112eb1f1b988e681e38349b27292f58c7 Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Fri, 24 Jul 2015 12:33:56 +0200 Subject: [PATCH 06/11] No need to find replica copy when index is created There is no need to try and go fetch replica copies for best allocation when the index is created --- .../gateway/ReplicaShardAllocator.java | 9 +++++ .../gateway/ReplicaShardAllocatorTests.java | 40 ++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/gateway/ReplicaShardAllocator.java b/core/src/main/java/org/elasticsearch/gateway/ReplicaShardAllocator.java index 4bf6be893e3..6ab016be247 100644 --- a/core/src/main/java/org/elasticsearch/gateway/ReplicaShardAllocator.java +++ b/core/src/main/java/org/elasticsearch/gateway/ReplicaShardAllocator.java @@ -70,6 +70,10 @@ public abstract class ReplicaShardAllocator extends AbstractComponent { if (shard.relocatingNodeId() != null) { continue; } + // if we are allocating a replica because of index creation, no need to go and find a copy, there isn't one... + if (shard.allocatedPostIndexCreate() == false) { + continue; + } AsyncShardFetch.FetchResult shardStores = fetchData(shard, allocation); if (shardStores.hasData() == false) { @@ -116,6 +120,11 @@ public abstract class ReplicaShardAllocator extends AbstractComponent { continue; } + // if we are allocating a replica because of index creation, no need to go and find a copy, there isn't one... + if (shard.allocatedPostIndexCreate() == false) { + continue; + } + // pre-check if it can be allocated to any node that currently exists, so we won't list the store for it for nothing if (canBeAllocatedToAtLeastOneNode(shard, allocation) == false) { logger.trace("{}: ignoring allocation, can't be allocated on any node", shard); diff --git a/core/src/test/java/org/elasticsearch/gateway/ReplicaShardAllocatorTests.java b/core/src/test/java/org/elasticsearch/gateway/ReplicaShardAllocatorTests.java index 97c7ecdcfd9..61dc147c9d3 100644 --- a/core/src/test/java/org/elasticsearch/gateway/ReplicaShardAllocatorTests.java +++ b/core/src/test/java/org/elasticsearch/gateway/ReplicaShardAllocatorTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.gateway; +import com.carrotsearch.randomizedtesting.generators.RandomPicks; import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -42,8 +43,10 @@ import org.junit.Before; import org.junit.Test; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import static org.hamcrest.Matchers.equalTo; @@ -75,6 +78,33 @@ public class ReplicaShardAllocatorTests extends ElasticsearchAllocationTestCase assertThat(allocation.routingNodes().unassigned().ignored().get(0).shardId(), equalTo(shardId)); } + /** + * Verifies that on index creation, we don't go and fetch data, but keep the replica shard unassigned to let + * the shard allocator to allocate it. There isn't a copy around to find anyhow. + */ + @Test + public void testNoAsyncFetchOnIndexCreation() { + RoutingAllocation allocation = onePrimaryOnNode1And1Replica(yesAllocationDeciders(), Settings.EMPTY, UnassignedInfo.Reason.INDEX_CREATED); + testAllocator.clean(); + testAllocator.allocateUnassigned(allocation); + assertThat(testAllocator.getFetchDataCalledAndClean(), equalTo(false)); + assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED).size(), equalTo(1)); + assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED).get(0).shardId(), equalTo(shardId)); + } + + /** + * Verifies that for anything but index creation, fetch data ends up being called, since we need to go and try + * and find a better copy for the shard. + */ + @Test + public void testAsyncFetchOnAnythingButIndexCreation() { + UnassignedInfo.Reason reason = RandomPicks.randomFrom(getRandom(), EnumSet.complementOf(EnumSet.of(UnassignedInfo.Reason.INDEX_CREATED))); + RoutingAllocation allocation = onePrimaryOnNode1And1Replica(yesAllocationDeciders(), Settings.EMPTY, reason); + testAllocator.clean(); + testAllocator.allocateUnassigned(allocation); + assertThat("failed with reason " + reason, testAllocator.getFetchDataCalledAndClean(), equalTo(true)); + } + /** * Verifies that when there is a full match (syncId and files) we allocate it to matching node. */ @@ -253,7 +283,7 @@ public class ReplicaShardAllocatorTests extends ElasticsearchAllocationTestCase } private RoutingAllocation onePrimaryOnNode1And1Replica(AllocationDeciders deciders) { - return onePrimaryOnNode1And1Replica(deciders, Settings.EMPTY, UnassignedInfo.Reason.INDEX_CREATED); + return onePrimaryOnNode1And1Replica(deciders, Settings.EMPTY, UnassignedInfo.Reason.CLUSTER_RECOVERED); } private RoutingAllocation onePrimaryOnNode1And1Replica(AllocationDeciders deciders, Settings settings, UnassignedInfo.Reason reason) { @@ -283,7 +313,7 @@ public class ReplicaShardAllocatorTests extends ElasticsearchAllocationTestCase .add(IndexRoutingTable.builder(shardId.getIndex()) .addIndexShard(new IndexShardRoutingTable.Builder(shardId) .addShard(TestShardRouting.newShardRouting(shardId.getIndex(), shardId.getId(), node1.id(), true, ShardRoutingState.STARTED, 10)) - .addShard(TestShardRouting.newShardRouting(shardId.getIndex(), shardId.getId(), node2.id(), false, ShardRoutingState.INITIALIZING, 10)) + .addShard(TestShardRouting.newShardRouting(shardId.getIndex(), shardId.getId(), node2.id(), null, null, false, ShardRoutingState.INITIALIZING, 10, new UnassignedInfo(UnassignedInfo.Reason.CLUSTER_RECOVERED, null))) .build()) ) .build(); @@ -297,6 +327,7 @@ public class ReplicaShardAllocatorTests extends ElasticsearchAllocationTestCase class TestAllocator extends ReplicaShardAllocator { private Map data = null; + private AtomicBoolean fetchDataCalled = new AtomicBoolean(false); public TestAllocator() { super(Settings.EMPTY); @@ -310,6 +341,10 @@ public class ReplicaShardAllocatorTests extends ElasticsearchAllocationTestCase data = new HashMap<>(); } + public boolean getFetchDataCalledAndClean() { + return fetchDataCalled.getAndSet(false); + } + public TestAllocator addData(DiscoveryNode node, boolean allocated, String syncId, StoreFileMetaData... files) { if (data == null) { data = new HashMap<>(); @@ -328,6 +363,7 @@ public class ReplicaShardAllocatorTests extends ElasticsearchAllocationTestCase @Override protected AsyncShardFetch.FetchResult fetchData(ShardRouting shard, RoutingAllocation allocation) { + fetchDataCalled.set(true); Map tData = null; if (data != null) { tData = new HashMap<>(); From b8c2f05ff54e8236a0b9eb76428e564d60884a23 Mon Sep 17 00:00:00 2001 From: Ruflin Date: Mon, 27 Jul 2015 09:11:04 +0200 Subject: [PATCH 07/11] Revert change to set longTo to "MAX_IP -1" and improve test suite to check for range --- .../bucket/range/ipv4/IPv4RangeBuilder.java | 3 +- .../aggregations/bucket/IPv4RangeTests.java | 72 ++++++++++--------- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IPv4RangeBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IPv4RangeBuilder.java index 86eb7eb2bcb..6d17bee5764 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IPv4RangeBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IPv4RangeBuilder.java @@ -151,8 +151,9 @@ public class IPv4RangeBuilder extends AbstractRangeBuilder { int to = from + (~mask); long longTo = intIpToLongIp(to) + 1; // we have to +1 here as the range is non-inclusive on the "to" side + if (longTo == InternalIPv4Range.MAX_IP) { - longTo = InternalIPv4Range.MAX_IP - 1; + longTo = -1; } return new long[] { longFrom, longTo }; diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java index 244df1303ea..bf707a29972 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java @@ -872,8 +872,8 @@ public class IPv4RangeTests extends ElasticsearchIntegrationTest { } @Test - public void cidr0Mask0() { - SearchResponse response = client().prepareSearch("idx_unmapped") + public void mask0() { + SearchResponse response = client().prepareSearch("idx") .addAggregation(ipRange("range") .field("ip") .addMaskRange("0.0.0.0/0")) @@ -881,7 +881,6 @@ public class IPv4RangeTests extends ElasticsearchIntegrationTest { assertSearchResponse(response); - Range range = response.getAggregations().get("range"); assertThat(range, notNullValue()); assertThat(range.getName(), equalTo("range")); @@ -890,53 +889,58 @@ public class IPv4RangeTests extends ElasticsearchIntegrationTest { Range.Bucket bucket = buckets.get(0); assertThat((String) bucket.getKey(), equalTo("0.0.0.0/0")); - assertThat(bucket.getFromAsString(), nullValue()); - assertThat(bucket.getToAsString(), equalTo("255.255.255.255")); + assertThat(bucket.getToAsString(), nullValue()); + assertThat(((Number) bucket.getTo()).doubleValue(), equalTo(Double.POSITIVE_INFINITY)); + assertEquals(255l, bucket.getDocCount()); } + @Test - public void cidr0Mask1() { - SearchResponse response = client().prepareSearch("idx_unmapped") + public void mask0SpecialIps() throws Exception { + assertAcked(prepareCreate("idx_range") + .addMapping("type", "ip", "type=ip", "ips", "type=ip")); + IndexRequestBuilder[] builders = new IndexRequestBuilder[4]; + + builders[0] = client().prepareIndex("idx_range", "type").setSource(jsonBuilder() + .startObject() + .field("ip", "0.0.0.0") + .endObject()); + + builders[1] = client().prepareIndex("idx_range", "type").setSource(jsonBuilder() + .startObject() + .field("ip", "0.0.0.255") + .endObject()); + + builders[2] = client().prepareIndex("idx_range", "type").setSource(jsonBuilder() + .startObject() + .field("ip", "255.255.255.0") + .endObject()); + + builders[3] = client().prepareIndex("idx_range", "type").setSource(jsonBuilder() + .startObject() + .field("ip", "255.255.255.255") + .endObject()); + + indexRandom(true, builders); + ensureSearchable(); + + SearchResponse response = client().prepareSearch("idx_range") .addAggregation(ipRange("range") .field("ip") - .addMaskRange("0.0.0.0/1")) + .addMaskRange("0.0.0.0/0")) .execute().actionGet(); assertSearchResponse(response); - Range range = response.getAggregations().get("range"); + assertThat(range, notNullValue()); assertThat(range.getName(), equalTo("range")); List buckets = range.getBuckets(); assertThat(range.getBuckets().size(), equalTo(1)); Range.Bucket bucket = buckets.get(0); - assertThat((String) bucket.getKey(), equalTo("0.0.0.0/1")); - assertThat(bucket.getFromAsString(), nullValue()); - assertThat(bucket.getToAsString(), equalTo("128.0.0.0")); - } - - @Test - public void cidr0Mask2() { - SearchResponse response = client().prepareSearch("idx_unmapped") - .addAggregation(ipRange("range") - .field("ip") - .addMaskRange("0.0.0.0/2")) - .execute().actionGet(); - - assertSearchResponse(response); - - - Range range = response.getAggregations().get("range"); - assertThat(range, notNullValue()); - assertThat(range.getName(), equalTo("range")); - List buckets = range.getBuckets(); - assertThat(range.getBuckets().size(), equalTo(1)); - - Range.Bucket bucket = buckets.get(0); - assertThat(bucket.getFromAsString(), nullValue()); - assertThat(bucket.getToAsString(), equalTo("64.0.0.0")); + assertEquals(4l, bucket.getDocCount()); } } From 91d17892581e1df067e7fbdb8f760ae220554dbc Mon Sep 17 00:00:00 2001 From: Ruflin Date: Mon, 27 Jul 2015 10:39:04 +0200 Subject: [PATCH 08/11] Move index creation to test setup method --- .../aggregations/bucket/IPv4RangeTests.java | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java index bf707a29972..fd7db94b3b9 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/IPv4RangeTests.java @@ -25,7 +25,6 @@ import org.elasticsearch.script.Script; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.range.Range; import org.elasticsearch.search.aggregations.bucket.range.Range.Bucket; -import org.elasticsearch.search.aggregations.bucket.range.ipv4.IPv4RangeBuilder; import org.elasticsearch.search.aggregations.metrics.max.Max; import org.elasticsearch.search.aggregations.metrics.sum.Sum; import org.elasticsearch.test.ElasticsearchIntegrationTest; @@ -84,6 +83,33 @@ public class IPv4RangeTests extends ElasticsearchIntegrationTest { } indexRandom(true, builders.toArray(new IndexRequestBuilder[builders.size()])); } + { + assertAcked(prepareCreate("range_idx") + .addMapping("type", "ip", "type=ip", "ips", "type=ip")); + IndexRequestBuilder[] builders = new IndexRequestBuilder[4]; + + builders[0] = client().prepareIndex("range_idx", "type").setSource(jsonBuilder() + .startObject() + .field("ip", "0.0.0.0") + .endObject()); + + builders[1] = client().prepareIndex("range_idx", "type").setSource(jsonBuilder() + .startObject() + .field("ip", "0.0.0.255") + .endObject()); + + builders[2] = client().prepareIndex("range_idx", "type").setSource(jsonBuilder() + .startObject() + .field("ip", "255.255.255.0") + .endObject()); + + builders[3] = client().prepareIndex("range_idx", "type").setSource(jsonBuilder() + .startObject() + .field("ip", "255.255.255.255") + .endObject()); + + indexRandom(true, builders); + } ensureSearchable(); } @@ -897,35 +923,9 @@ public class IPv4RangeTests extends ElasticsearchIntegrationTest { @Test - public void mask0SpecialIps() throws Exception { - assertAcked(prepareCreate("idx_range") - .addMapping("type", "ip", "type=ip", "ips", "type=ip")); - IndexRequestBuilder[] builders = new IndexRequestBuilder[4]; + public void mask0SpecialIps() { - builders[0] = client().prepareIndex("idx_range", "type").setSource(jsonBuilder() - .startObject() - .field("ip", "0.0.0.0") - .endObject()); - - builders[1] = client().prepareIndex("idx_range", "type").setSource(jsonBuilder() - .startObject() - .field("ip", "0.0.0.255") - .endObject()); - - builders[2] = client().prepareIndex("idx_range", "type").setSource(jsonBuilder() - .startObject() - .field("ip", "255.255.255.0") - .endObject()); - - builders[3] = client().prepareIndex("idx_range", "type").setSource(jsonBuilder() - .startObject() - .field("ip", "255.255.255.255") - .endObject()); - - indexRandom(true, builders); - ensureSearchable(); - - SearchResponse response = client().prepareSearch("idx_range") + SearchResponse response = client().prepareSearch("range_idx") .addAggregation(ipRange("range") .field("ip") .addMaskRange("0.0.0.0/0")) From 8e931d57672be67bd613469a32ff554f9e92400c Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 24 Jul 2015 18:37:17 +0200 Subject: [PATCH 09/11] Query DSL: don't cache type filter in DocumentMapper If we cache the type filter and we e.g. set its boost which is now settable on all queries, the boost will change for all subsequent queries. We should rather create a new query every time. --- .../java/org/elasticsearch/index/mapper/DocumentMapper.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index 989f1f2c813..1fbd0f2a36c 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -168,8 +168,6 @@ public class DocumentMapper implements ToXContent { private boolean hasNestedObjects = false; - private final Query typeFilter; - private final ReleasableLock mappingWriteLock; private final ReentrantReadWriteLock mappingLock; @@ -190,7 +188,6 @@ public class DocumentMapper implements ToXContent { meta); this.documentParser = new DocumentParser(indexSettings, docMapperParser, this, new ReleasableLock(mappingLock.readLock())); - this.typeFilter = typeMapper().fieldType().termQuery(type, null); this.mappingWriteLock = new ReleasableLock(mappingLock.writeLock()); this.mappingLock = mappingLock; @@ -307,7 +304,7 @@ public class DocumentMapper implements ToXContent { } public Query typeFilter() { - return this.typeFilter; + return typeMapper().fieldType().termQuery(type, null); } public boolean hasNestedObjects() { From 2713e903ab140629020e26ee43415fd30ba23741 Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Mon, 15 Jun 2015 16:16:18 +0200 Subject: [PATCH 10/11] [TEST] remove redundant tests and move to different suite Some of the test for meta data are redundant. Also, since they somewhat test service disruptions (start master with empty data folder) we might move them to DiscoveryWithServiceDisruptionsTests. Also, this commit adds a test for https://github.com/elastic/elasticsearch/issues/11665 --- .../cluster/ClusterChangedEvent.java | 7 +- .../DiscoveryWithServiceDisruptionsTests.java | 52 ++- .../gateway/MetaDataWriteDataNodesTests.java | 312 +++++------------- .../test/InternalTestCluster.java | 62 +++- 4 files changed, 205 insertions(+), 228 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/ClusterChangedEvent.java b/core/src/main/java/org/elasticsearch/cluster/ClusterChangedEvent.java index e3469abde3e..1ceb822abfd 100644 --- a/core/src/main/java/org/elasticsearch/cluster/ClusterChangedEvent.java +++ b/core/src/main/java/org/elasticsearch/cluster/ClusterChangedEvent.java @@ -110,8 +110,11 @@ public class ClusterChangedEvent { // is actually supposed to be deleted or imported as dangling instead. for example a new master might not have // the index in its cluster state because it was started with an empty data folder and in this case we want to // import as dangling. we check here for new master too to be on the safe side in this case. - // norelease because we are not sure this is actually a good solution - // See discussion on https://github.com/elastic/elasticsearch/pull/9952 + // This means that under certain conditions deleted indices might be reimported if a master fails while the deletion + // request is issued and a node receives the cluster state that would trigger the deletion from the new master. + // See test MetaDataWriteDataNodesTests.testIndicesDeleted() + // See discussion on https://github.com/elastic/elasticsearch/pull/9952 and + // https://github.com/elastic/elasticsearch/issues/11665 if (hasNewMaster() || previousState == null) { return ImmutableList.of(); } diff --git a/core/src/test/java/org/elasticsearch/discovery/DiscoveryWithServiceDisruptionsTests.java b/core/src/test/java/org/elasticsearch/discovery/DiscoveryWithServiceDisruptionsTests.java index 0755b9ab6fd..8a758fe83d6 100644 --- a/core/src/test/java/org/elasticsearch/discovery/DiscoveryWithServiceDisruptionsTests.java +++ b/core/src/test/java/org/elasticsearch/discovery/DiscoveryWithServiceDisruptionsTests.java @@ -56,6 +56,7 @@ import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.transport.*; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import java.io.IOException; @@ -65,6 +66,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope; import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; @@ -237,7 +239,9 @@ public class DiscoveryWithServiceDisruptionsTests extends ElasticsearchIntegrati } - /** Verify that nodes fault detection works after master (re) election */ + /** + * Verify that nodes fault detection works after master (re) election + */ @Test public void testNodesFDAfterMasterReelection() throws Exception { startCluster(4); @@ -414,7 +418,7 @@ public class DiscoveryWithServiceDisruptionsTests extends ElasticsearchIntegrati /** * Test that we do not loose document whose indexing request was successful, under a randomly selected disruption scheme * We also collect & report the type of indexing failures that occur. - * + *

* This test is a superset of tests run in the Jepsen test suite, with the exception of versioned updates */ @Test @@ -948,6 +952,50 @@ public class DiscoveryWithServiceDisruptionsTests extends ElasticsearchIntegrati ensureStableCluster(3); } + @Test + public void testIndexImportedFromDataOnlyNodesIfMasterLostDataFolder() throws Exception { + // test for https://github.com/elastic/elasticsearch/issues/8823 + configureCluster(2, 1); + String masterNode = internalCluster().startMasterOnlyNode(Settings.EMPTY); + internalCluster().startDataOnlyNode(Settings.EMPTY); + + ensureStableCluster(2); + assertAcked(prepareCreate("index").setSettings(Settings.builder().put("index.number_of_replicas", 0))); + index("index", "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); + ensureGreen(); + + internalCluster().restartNode(masterNode, new InternalTestCluster.RestartCallback() { + public boolean clearData(String nodeName) { + return true; + } + }); + + ensureGreen("index"); + assertTrue(client().prepareGet("index", "doc", "1").get().isExists()); + } + + // tests if indices are really deleted even if a master transition inbetween + @Ignore("https://github.com/elastic/elasticsearch/issues/11665") + @Test + public void testIndicesDeleted() throws Exception { + configureCluster(3, 2); + Future> masterNodes= internalCluster().startMasterOnlyNodesAsync(2); + Future dataNode = internalCluster().startDataOnlyNodeAsync(); + dataNode.get(); + masterNodes.get(); + ensureStableCluster(3); + assertAcked(prepareCreate("test")); + ensureYellow(); + + String masterNode1 = internalCluster().getMasterName(); + NetworkPartition networkPartition = new NetworkUnresponsivePartition(masterNode1, dataNode.get(), getRandom()); + internalCluster().setDisruptionScheme(networkPartition); + networkPartition.startDisrupting(); + internalCluster().client(masterNode1).admin().indices().prepareDelete("test").setTimeout("1s").get(); + internalCluster().restartNode(masterNode1, InternalTestCluster.EMPTY_CALLBACK); + ensureYellow(); + assertFalse(client().admin().indices().prepareExists("test").get().isExists()); + } protected NetworkPartition addRandomPartition() { NetworkPartition partition; diff --git a/core/src/test/java/org/elasticsearch/gateway/MetaDataWriteDataNodesTests.java b/core/src/test/java/org/elasticsearch/gateway/MetaDataWriteDataNodesTests.java index 30608cf1a8d..f4fcc088710 100644 --- a/core/src/test/java/org/elasticsearch/gateway/MetaDataWriteDataNodesTests.java +++ b/core/src/test/java/org/elasticsearch/gateway/MetaDataWriteDataNodesTests.java @@ -21,6 +21,7 @@ package org.elasticsearch.gateway; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import com.google.common.base.Predicate; +import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -33,227 +34,81 @@ import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope; import org.elasticsearch.test.InternalTestCluster; import org.junit.Test; -import java.io.IOException; import java.util.LinkedHashMap; +import java.util.concurrent.Future; -import static org.elasticsearch.client.Requests.clusterHealthRequest; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope; +import static org.elasticsearch.test.InternalTestCluster.RestartCallback; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.equalTo; /** * */ +@LuceneTestCase.Slow @ClusterScope(scope = Scope.TEST, numDataNodes = 0) public class MetaDataWriteDataNodesTests extends ElasticsearchIntegrationTest { @Test public void testMetaWrittenAlsoOnDataNode() throws Exception { - // this test checks that index state is written on data only nodes - String masterNodeName = startMasterNode(); - String redNode = startDataNode("red"); - assertAcked(prepareCreate("test").setSettings(Settings.builder().put("index.number_of_replicas", 0))); + // this test checks that index state is written on data only nodes if they have a shard allocated + String masterNode = internalCluster().startMasterOnlyNode(Settings.EMPTY); + String dataNode = internalCluster().startDataOnlyNode(Settings.EMPTY); + assertAcked(prepareCreate("test").setSettings("index.number_of_replicas", 0)); index("test", "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); ensureGreen("test"); - assertIndexInMetaState(redNode, "test"); - assertIndexInMetaState(masterNodeName, "test"); - //stop master node and start again with an empty data folder - ((InternalTestCluster) cluster()).stopCurrentMasterNode(); - String newMasterNode = startMasterNode(); - ensureGreen("test"); - // check for meta data - assertIndexInMetaState(redNode, "test"); - assertIndexInMetaState(newMasterNode, "test"); - // check if index and doc is still there - ensureGreen("test"); - assertTrue(client().prepareGet("test", "doc", "1").get().isExists()); - } - - @Test - public void testMetaWrittenOnlyForIndicesOnNodesThatHaveAShard() throws Exception { - // this test checks that the index state is only written to a data only node if they have a shard of that index allocated on the node - String masterNode = startMasterNode(); - String blueNode = startDataNode("blue"); - String redNode = startDataNode("red"); - - assertAcked(prepareCreate("blue_index").setSettings(Settings.builder().put("index.number_of_replicas", 0).put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "color", "blue"))); - index("blue_index", "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); - assertAcked(prepareCreate("red_index").setSettings(Settings.builder().put("index.number_of_replicas", 0).put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "color", "red"))); - index("red_index", "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); - ensureGreen(); - assertIndexNotInMetaState(blueNode, "red_index"); - assertIndexNotInMetaState(redNode, "blue_index"); - assertIndexInMetaState(blueNode, "blue_index"); - assertIndexInMetaState(redNode, "red_index"); - assertIndexInMetaState(masterNode, "red_index"); - assertIndexInMetaState(masterNode, "blue_index"); - - // not the index state for blue_index should only be written on blue_node and the for red_index only on red_node - // we restart red node and master but with empty data folders - stopNode(redNode); - ((InternalTestCluster) cluster()).stopCurrentMasterNode(); - masterNode = startMasterNode(); - redNode = startDataNode("red"); - - ensureGreen(); - assertIndexNotInMetaState(blueNode, "red_index"); - assertIndexInMetaState(blueNode, "blue_index"); - assertIndexNotInMetaState(redNode, "red_index"); - assertIndexNotInMetaState(redNode, "blue_index"); - assertIndexNotInMetaState(masterNode, "red_index"); - assertIndexInMetaState(masterNode, "blue_index"); - // check that blue index is still there - assertFalse(client().admin().indices().prepareExists("red_index").get().isExists()); - assertTrue(client().prepareGet("blue_index", "doc", "1").get().isExists()); - // red index should be gone - // if the blue node had stored the index state then cluster health would be red and red_index would exist - assertFalse(client().admin().indices().prepareExists("red_index").get().isExists()); - + assertIndexInMetaState(dataNode, "test"); + assertIndexInMetaState(masterNode, "test"); } @Test public void testMetaIsRemovedIfAllShardsFromIndexRemoved() throws Exception { // this test checks that the index state is removed from a data only node once all shards have been allocated away from it - String masterNode = startMasterNode(); - String blueNode = startDataNode("blue"); - String redNode = startDataNode("red"); - - // create blue_index on blue_node and same for red - client().admin().cluster().health(clusterHealthRequest().waitForYellowStatus().waitForNodes("3")).get(); - assertAcked(prepareCreate("blue_index").setSettings(Settings.builder().put("index.number_of_replicas", 0).put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "color", "blue"))); - index("blue_index", "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); - assertAcked(prepareCreate("red_index").setSettings(Settings.builder().put("index.number_of_replicas", 0).put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "color", "red"))); - index("red_index", "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); + String masterNode = internalCluster().startMasterOnlyNode(Settings.EMPTY); + Future nodeName1 = internalCluster().startDataOnlyNodeAsync(); + Future nodeName2 = internalCluster().startDataOnlyNodeAsync(); + String node1 = nodeName1.get(); + String node2 = nodeName2.get(); + String index = "index"; + assertAcked(prepareCreate(index).setSettings(Settings.builder().put("index.number_of_replicas", 0).put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "_name", node1))); + index(index, "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); ensureGreen(); - assertIndexNotInMetaState(redNode, "blue_index"); - assertIndexNotInMetaState(blueNode, "red_index"); - assertIndexInMetaState(redNode, "red_index"); - assertIndexInMetaState(blueNode, "blue_index"); - assertIndexInMetaState(masterNode, "red_index"); - assertIndexInMetaState(masterNode, "blue_index"); + assertIndexInMetaState(node1, index); + assertIndexNotInMetaState(node2, index); + assertIndexInMetaState(masterNode, index); - // now relocate blue_index to red_node and red_index to blue_node - logger.debug("relocating indices..."); - client().admin().indices().prepareUpdateSettings("blue_index").setSettings(Settings.builder().put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "color", "red")).get(); - client().admin().indices().prepareUpdateSettings("red_index").setSettings(Settings.builder().put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "color", "blue")).get(); + logger.debug("relocating index..."); + client().admin().indices().prepareUpdateSettings(index).setSettings(Settings.builder().put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "_name", node2)).get(); client().admin().cluster().prepareHealth().setWaitForRelocatingShards(0).get(); ensureGreen(); - assertIndexNotInMetaState(redNode, "red_index"); - assertIndexNotInMetaState(blueNode, "blue_index"); - assertIndexInMetaState(redNode, "blue_index"); - assertIndexInMetaState(blueNode, "red_index"); - assertIndexInMetaState(masterNode, "red_index"); - assertIndexInMetaState(masterNode, "blue_index"); - - //at this point the blue_index is on red node and the red_index on blue node - // now, when we start red and master node again but without data folder, the red index should be gone but the blue index should initialize fine - stopNode(redNode); - ((InternalTestCluster) cluster()).stopCurrentMasterNode(); - masterNode = startMasterNode(); - redNode = startDataNode("red"); - ensureGreen(); - assertIndexNotInMetaState(redNode, "blue_index"); - assertIndexNotInMetaState(blueNode, "blue_index"); - assertIndexNotInMetaState(redNode, "red_index"); - assertIndexInMetaState(blueNode, "red_index"); - assertIndexInMetaState(masterNode, "red_index"); - assertIndexNotInMetaState(masterNode, "blue_index"); - assertTrue(client().prepareGet("red_index", "doc", "1").get().isExists()); - // if the red_node had stored the index state then cluster health would be red and blue_index would exist - assertFalse(client().admin().indices().prepareExists("blue_index").get().isExists()); - } - - @Test - public void testMetaWrittenWhenIndexIsClosed() throws Exception { - String masterNode = startMasterNode(); - String redNodeDataPath = createTempDir().toString(); - String redNode = startDataNode("red", redNodeDataPath); - String blueNode = startDataNode("blue"); - // create red_index on red_node and same for red - client().admin().cluster().health(clusterHealthRequest().waitForYellowStatus().waitForNodes("3")).get(); - assertAcked(prepareCreate("red_index").setSettings(Settings.builder().put("index.number_of_replicas", 0).put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "color", "red"))); - index("red_index", "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); - - ensureGreen(); - assertIndexNotInMetaState(blueNode, "red_index"); - assertIndexInMetaState(redNode, "red_index"); - assertIndexInMetaState(masterNode, "red_index"); - - client().admin().indices().prepareClose("red_index").get(); - // close the index - ClusterStateResponse clusterStateResponse = client().admin().cluster().prepareState().get(); - assertThat(clusterStateResponse.getState().getMetaData().index("red_index").getState().name(), equalTo(IndexMetaData.State.CLOSE.name())); - - // restart master with empty data folder and maybe red node - boolean restartRedNode = randomBoolean(); - //at this point the red_index on red node - if (restartRedNode) { - stopNode(redNode); - } - ((InternalTestCluster) cluster()).stopCurrentMasterNode(); - masterNode = startMasterNode(); - if (restartRedNode) { - redNode = startDataNode("red", redNodeDataPath); - } - - ensureGreen("red_index"); - assertIndexNotInMetaState(blueNode, "red_index"); - assertIndexInMetaState(redNode, "red_index"); - assertIndexInMetaState(masterNode, "red_index"); - clusterStateResponse = client().admin().cluster().prepareState().get(); - assertThat(clusterStateResponse.getState().getMetaData().index("red_index").getState().name(), equalTo(IndexMetaData.State.CLOSE.name())); - - // open the index again - client().admin().indices().prepareOpen("red_index").get(); - clusterStateResponse = client().admin().cluster().prepareState().get(); - assertThat(clusterStateResponse.getState().getMetaData().index("red_index").getState().name(), equalTo(IndexMetaData.State.OPEN.name())); - // restart again - ensureGreen(); - if (restartRedNode) { - stopNode(redNode); - } - ((InternalTestCluster) cluster()).stopCurrentMasterNode(); - masterNode = startMasterNode(); - if (restartRedNode) { - redNode = startDataNode("red", redNodeDataPath); - } - ensureGreen("red_index"); - assertIndexNotInMetaState(blueNode, "red_index"); - assertIndexInMetaState(redNode, "red_index"); - assertIndexInMetaState(masterNode, "red_index"); - clusterStateResponse = client().admin().cluster().prepareState().get(); - assertThat(clusterStateResponse.getState().getMetaData().index("red_index").getState().name(), equalTo(IndexMetaData.State.OPEN.name())); - assertTrue(client().prepareGet("red_index", "doc", "1").get().isExists()); + assertIndexNotInMetaState(node1, index); + assertIndexInMetaState(node2, index); + assertIndexInMetaState(masterNode, index); } @Test public void testMetaWrittenWhenIndexIsClosedAndMetaUpdated() throws Exception { - String masterNode = startMasterNode(); - String redNodeDataPath = createTempDir().toString(); - String redNode = startDataNode("red", redNodeDataPath); - // create red_index on red_node and same for red - client().admin().cluster().health(clusterHealthRequest().waitForYellowStatus().waitForNodes("2")).get(); - assertAcked(prepareCreate("red_index").setSettings(Settings.builder().put("index.number_of_replicas", 0).put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "color", "red"))); - index("red_index", "doc", "1", jsonBuilder().startObject().field("text", "some text").endObject()); + String masterNode = internalCluster().startMasterOnlyNode(Settings.EMPTY); + final String dataNode = internalCluster().startDataOnlyNode(Settings.EMPTY); - logger.info("--> wait for green red_index"); + final String index = "index"; + assertAcked(prepareCreate(index).setSettings(Settings.builder().put("index.number_of_replicas", 0))); + logger.info("--> wait for green index"); ensureGreen(); - logger.info("--> wait for meta state written for red_index"); - assertIndexInMetaState(redNode, "red_index"); - assertIndexInMetaState(masterNode, "red_index"); + logger.info("--> wait for meta state written for index"); + assertIndexInMetaState(dataNode, index); + assertIndexInMetaState(masterNode, index); - logger.info("--> close red_index"); - client().admin().indices().prepareClose("red_index").get(); + logger.info("--> close index"); + client().admin().indices().prepareClose(index).get(); // close the index ClusterStateResponse clusterStateResponse = client().admin().cluster().prepareState().get(); - assertThat(clusterStateResponse.getState().getMetaData().index("red_index").getState().name(), equalTo(IndexMetaData.State.CLOSE.name())); + assertThat(clusterStateResponse.getState().getMetaData().index(index).getState().name(), equalTo(IndexMetaData.State.CLOSE.name())); - logger.info("--> restart red node"); - stopNode(redNode); - redNode = startDataNode("red", redNodeDataPath); - client().admin().indices().preparePutMapping("red_index").setType("doc").setSource(jsonBuilder().startObject() + // update the mapping. this should cause the new meta data to be written although index is closed + client().admin().indices().preparePutMapping(index).setType("doc").setSource(jsonBuilder().startObject() .startObject("properties") .startObject("integer_field") .field("type", "integer") @@ -261,45 +116,54 @@ public class MetaDataWriteDataNodesTests extends ElasticsearchIntegrationTest { .endObject() .endObject()).get(); - GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("red_index").addTypes("doc").get(); - assertNotNull(((LinkedHashMap) (getMappingsResponse.getMappings().get("red_index").get("doc").getSourceAsMap().get("properties"))).get("integer_field")); - // restart master with empty data folder and maybe red node - ((InternalTestCluster) cluster()).stopCurrentMasterNode(); - masterNode = startMasterNode(); + GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes("doc").get(); + assertNotNull(((LinkedHashMap) (getMappingsResponse.getMappings().get(index).get("doc").getSourceAsMap().get("properties"))).get("integer_field")); - ensureGreen("red_index"); - assertIndexInMetaState(redNode, "red_index"); - assertIndexInMetaState(masterNode, "red_index"); - clusterStateResponse = client().admin().cluster().prepareState().get(); - assertThat(clusterStateResponse.getState().getMetaData().index("red_index").getState().name(), equalTo(IndexMetaData.State.CLOSE.name())); - getMappingsResponse = client().admin().indices().prepareGetMappings("red_index").addTypes("doc").get(); - assertNotNull(((LinkedHashMap) (getMappingsResponse.getMappings().get("red_index").get("doc").getSourceAsMap().get("properties"))).get("integer_field")); + // make sure it was also written on red node although index is closed + ImmutableOpenMap indicesMetaData = getIndicesMetaDataOnNode(dataNode); + assertNotNull(((LinkedHashMap) (indicesMetaData.get(index).getMappings().get("doc").getSourceAsMap().get("properties"))).get("integer_field")); + assertThat(indicesMetaData.get(index).state(), equalTo(IndexMetaData.State.CLOSE)); - } + /** + * Try the same and see if this also works if node was just restarted. + * Each node holds an array of indices it knows of and checks if it should + * write new meta data by looking up in this array. We need it because if an + * index is closed it will not appear in the shard routing and we therefore + * need to keep track of what we wrote before. However, when the node is + * restarted this array is empty and we have to fill it before we decide + * what we write. This is why I explicitly test for it. + */ + internalCluster().restartNode(dataNode, new RestartCallback()); + client().admin().indices().preparePutMapping(index).setType("doc").setSource(jsonBuilder().startObject() + .startObject("properties") + .startObject("float_field") + .field("type", "float") + .endObject() + .endObject() + .endObject()).get(); - private String startDataNode(String color) { - return startDataNode(color, createTempDir().toString()); - } + getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes("doc").get(); + assertNotNull(((LinkedHashMap) (getMappingsResponse.getMappings().get(index).get("doc").getSourceAsMap().get("properties"))).get("float_field")); - private String startDataNode(String color, String newDataPath) { - Settings.Builder settingsBuilder = Settings.builder() - .put("node.data", true) - .put("node.master", false) - .put("node.color", color) - .put("path.data", newDataPath); - return internalCluster().startNode(settingsBuilder.build()); - } + // make sure it was also written on red node although index is closed + indicesMetaData = getIndicesMetaDataOnNode(dataNode); + assertNotNull(((LinkedHashMap) (indicesMetaData.get(index).getMappings().get("doc").getSourceAsMap().get("properties"))).get("float_field")); + assertThat(indicesMetaData.get(index).state(), equalTo(IndexMetaData.State.CLOSE)); - private String startMasterNode() { - Settings.Builder settingsBuilder = Settings.builder() - .put("node.data", false) - .put("node.master", true) - .put("path.data", createTempDir().toString()); - return internalCluster().startNode(settingsBuilder.build()); - } - - private void stopNode(String name) throws IOException { - internalCluster().stopRandomNode(InternalTestCluster.nameFilter(name)); + // finally check that meta data is also written of index opened again + client().admin().indices().prepareOpen(index).get(); + assertBusy(new Runnable() { + @Override + public void run() { + try { + ImmutableOpenMap indicesMetaData = getIndicesMetaDataOnNode(dataNode); + assertThat(indicesMetaData.get(index).state(), equalTo(IndexMetaData.State.OPEN)); + } catch (Exception e) { + logger.info("caught exception while reading meta state: ", e); + fail(); + } + } + }); } protected void assertIndexNotInMetaState(String nodeName, String indexName) throws Exception { @@ -335,14 +199,18 @@ public class MetaDataWriteDataNodesTests extends ElasticsearchIntegrationTest { } private boolean metaStateExists(String nodeName, String indexName) throws Exception { - GatewayMetaState nodeMetaState = ((InternalTestCluster) cluster()).getInstance(GatewayMetaState.class, nodeName); - MetaData nodeMetaData = null; - nodeMetaData = nodeMetaState.loadMetaState(); - ImmutableOpenMap indices = nodeMetaData.getIndices(); + ImmutableOpenMap indices = getIndicesMetaDataOnNode(nodeName); boolean inMetaSate = false; for (ObjectObjectCursor index : indices) { inMetaSate = inMetaSate || index.key.equals(indexName); } return inMetaSate; } + + private ImmutableOpenMap getIndicesMetaDataOnNode(String nodeName) throws Exception { + GatewayMetaState nodeMetaState = ((InternalTestCluster) cluster()).getInstance(GatewayMetaState.class, nodeName); + MetaData nodeMetaData = null; + nodeMetaData = nodeMetaState.loadMetaState(); + return nodeMetaData.getIndices(); + } } diff --git a/core/src/test/java/org/elasticsearch/test/InternalTestCluster.java b/core/src/test/java/org/elasticsearch/test/InternalTestCluster.java index ac785c1601d..5204f0b456d 100644 --- a/core/src/test/java/org/elasticsearch/test/InternalTestCluster.java +++ b/core/src/test/java/org/elasticsearch/test/InternalTestCluster.java @@ -1286,6 +1286,18 @@ public final class InternalTestCluster extends TestCluster { } } + /** + * Restarts a node and calls the callback during restart. + */ + public void restartNode(String nodeName, RestartCallback callback) throws Exception { + ensureOpen(); + NodeAndClient nodeAndClient = nodes.get(nodeName); + if (nodeAndClient != null) { + logger.info("Restarting node [{}] ", nodeAndClient.name); + nodeAndClient.restart(callback); + } + } + private void restartAllNodes(boolean rollingRestart, RestartCallback callback) throws Exception { ensureOpen(); List toRemove = new ArrayList<>(); @@ -1343,7 +1355,7 @@ public final class InternalTestCluster extends TestCluster { } - private static final RestartCallback EMPTY_CALLBACK = new RestartCallback() { + public static final RestartCallback EMPTY_CALLBACK = new RestartCallback() { @Override public Settings onNodeStopped(String node) { return null; @@ -1468,6 +1480,52 @@ public final class InternalTestCluster extends TestCluster { return buildNode.name; } + public synchronized ListenableFuture> startMasterOnlyNodesAsync(int numNodes) { + return startMasterOnlyNodesAsync(numNodes, Settings.EMPTY); + } + + public synchronized ListenableFuture> startMasterOnlyNodesAsync(int numNodes, Settings settings) { + Settings settings1 = Settings.builder().put(settings).put("node.master", true).put("node.data", false).build(); + return startNodesAsync(numNodes, settings1, Version.CURRENT); + } + + public synchronized ListenableFuture> startDataOnlyNodesAsync(int numNodes) { + return startDataOnlyNodesAsync(numNodes, Settings.EMPTY); + } + + public synchronized ListenableFuture> startDataOnlyNodesAsync(int numNodes, Settings settings) { + Settings settings1 = Settings.builder().put(settings).put("node.master", false).put("node.data", true).build(); + return startNodesAsync(numNodes, settings1, Version.CURRENT); + } + + public synchronized ListenableFuture startMasterOnlyNodeAsync() { + return startMasterOnlyNodeAsync(Settings.EMPTY); + } + + public synchronized ListenableFuture startMasterOnlyNodeAsync(Settings settings) { + Settings settings1 = Settings.builder().put(settings).put("node.master", true).put("node.data", false).build(); + return startNodeAsync(settings1, Version.CURRENT); + } + + public synchronized String startMasterOnlyNode(Settings settings) { + Settings settings1 = Settings.builder().put(settings).put("node.master", true).put("node.data", false).build(); + return startNode(settings1, Version.CURRENT); + } + + public synchronized ListenableFuture startDataOnlyNodeAsync() { + return startDataOnlyNodeAsync(Settings.EMPTY); + } + + public synchronized ListenableFuture startDataOnlyNodeAsync(Settings settings) { + Settings settings1 = Settings.builder().put(settings).put("node.master", false).put("node.data", true).build(); + return startNodeAsync(settings1, Version.CURRENT); + } + + public synchronized String startDataOnlyNode(Settings settings) { + Settings settings1 = Settings.builder().put(settings).put("node.master", false).put("node.data", true).build(); + return startNode(settings1, Version.CURRENT); + } + /** * Starts a node in an async manner with the given settings and returns future with its name. */ @@ -1726,7 +1784,7 @@ public final class InternalTestCluster extends TestCluster { * and / or {@link #fullRestart(InternalTestCluster.RestartCallback)} to execute actions at certain * stages of the restart. */ - public static abstract class RestartCallback { + public static class RestartCallback { /** * Executed once the give node name has been stopped. From e44c5ff70384751bfa9bd5224183c1bc8c68daa0 Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Thu, 23 Jul 2015 16:39:21 +0200 Subject: [PATCH 11/11] check if index is closed or was previously closed when gathering relevant indices to write meta state When an index is opened it will not be assigned to a node but also not have closed state anymore. Before we only checked if an index either is closed or assigned to the data node and therefore the change from close->open was not written. --- .../gateway/GatewayMetaState.java | 21 +++++++++++++------ .../gateway/GatewayMetaStateTests.java | 4 ++-- .../gateway/MetaDataWriteDataNodesTests.java | 21 +++++-------------- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java b/core/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java index 86d23ee4416..b927cbbde84 100644 --- a/core/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java +++ b/core/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java @@ -28,7 +28,8 @@ import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.metadata.MetaDataIndexUpgradeService; import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.routing.*; +import org.elasticsearch.cluster.routing.RoutingNode; +import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; @@ -98,6 +99,7 @@ public class GatewayMetaState extends AbstractComponent implements ClusterStateL @Override public void clusterChanged(ClusterChangedEvent event) { + Set relevantIndices = new HashSet<>(); final ClusterState state = event.state(); if (state.blocks().disableStatePersistence()) { @@ -148,7 +150,7 @@ public class GatewayMetaState extends AbstractComponent implements ClusterStateL } Iterable writeInfo; - relevantIndices = getRelevantIndices(event.state(), previouslyWrittenIndices); + relevantIndices = getRelevantIndices(event.state(), event.previousState(), previouslyWrittenIndices); writeInfo = resolveStatesToBeWritten(previouslyWrittenIndices, relevantIndices, previousMetaData, event.state().metaData()); // check and write changes in indices for (IndexMetaWriteInfo indexMetaWrite : writeInfo) { @@ -169,10 +171,10 @@ public class GatewayMetaState extends AbstractComponent implements ClusterStateL } } - public static Set getRelevantIndices(ClusterState state, ImmutableSet previouslyWrittenIndices) { + public static Set getRelevantIndices(ClusterState state, ClusterState previousState,ImmutableSet previouslyWrittenIndices) { Set relevantIndices; if (isDataOnlyNode(state)) { - relevantIndices = getRelevantIndicesOnDataOnlyNode(state, previouslyWrittenIndices); + relevantIndices = getRelevantIndicesOnDataOnlyNode(state, previousState, previouslyWrittenIndices); } else if (state.nodes().localNode().masterNode() == true) { relevantIndices = getRelevantIndicesForMasterEligibleNode(state); } else { @@ -278,7 +280,7 @@ public class GatewayMetaState extends AbstractComponent implements ClusterStateL return indicesToWrite; } - public static Set getRelevantIndicesOnDataOnlyNode(ClusterState state, ImmutableSet previouslyWrittenIndices) { + public static Set getRelevantIndicesOnDataOnlyNode(ClusterState state, ClusterState previousState, ImmutableSet previouslyWrittenIndices) { RoutingNode newRoutingNode = state.getRoutingNodes().node(state.nodes().localNodeId()); if (newRoutingNode == null) { throw new IllegalStateException("cluster state does not contain this node - cannot write index meta state"); @@ -289,7 +291,14 @@ public class GatewayMetaState extends AbstractComponent implements ClusterStateL } // we have to check the meta data also: closed indices will not appear in the routing table, but we must still write the state if we have it written on disk previously for (IndexMetaData indexMetaData : state.metaData()) { - if (previouslyWrittenIndices.contains(indexMetaData.getIndex()) && state.metaData().getIndices().get(indexMetaData.getIndex()).state().equals(IndexMetaData.State.CLOSE)) { + boolean isOrWasClosed = indexMetaData.state().equals(IndexMetaData.State.CLOSE); + // if the index is open we might still have to write the state if it just transitioned from closed to open + // so we have to check for that as well. + IndexMetaData previousMetaData = previousState.metaData().getIndices().get(indexMetaData.getIndex()); + if (previousMetaData != null) { + isOrWasClosed = isOrWasClosed || previousMetaData.state().equals(IndexMetaData.State.CLOSE); + } + if (previouslyWrittenIndices.contains(indexMetaData.getIndex()) && isOrWasClosed) { indices.add(indexMetaData.getIndex()); } } diff --git a/core/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java b/core/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java index ffe4e716b5f..594dee7c57f 100644 --- a/core/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java +++ b/core/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java @@ -174,9 +174,9 @@ public class GatewayMetaStateTests extends ElasticsearchAllocationTestCase { if (stateInMemory) { inMemoryMetaData = event.previousState().metaData(); ImmutableSet.Builder relevantIndices = ImmutableSet.builder(); - oldIndicesList = relevantIndices.addAll(GatewayMetaState.getRelevantIndices(event.previousState(), oldIndicesList)).build(); + oldIndicesList = relevantIndices.addAll(GatewayMetaState.getRelevantIndices(event.previousState(), event.previousState(), oldIndicesList)).build(); } - Set newIndicesList = GatewayMetaState.getRelevantIndices(event.state(), oldIndicesList); + Set newIndicesList = GatewayMetaState.getRelevantIndices(event.state(),event.previousState(), oldIndicesList); // third, get the actual write info Iterator indices = GatewayMetaState.resolveStatesToBeWritten(oldIndicesList, newIndicesList, inMemoryMetaData, event.state().metaData()).iterator(); diff --git a/core/src/test/java/org/elasticsearch/gateway/MetaDataWriteDataNodesTests.java b/core/src/test/java/org/elasticsearch/gateway/MetaDataWriteDataNodesTests.java index f4fcc088710..afa3c5b44c1 100644 --- a/core/src/test/java/org/elasticsearch/gateway/MetaDataWriteDataNodesTests.java +++ b/core/src/test/java/org/elasticsearch/gateway/MetaDataWriteDataNodesTests.java @@ -124,14 +124,13 @@ public class MetaDataWriteDataNodesTests extends ElasticsearchIntegrationTest { assertNotNull(((LinkedHashMap) (indicesMetaData.get(index).getMappings().get("doc").getSourceAsMap().get("properties"))).get("integer_field")); assertThat(indicesMetaData.get(index).state(), equalTo(IndexMetaData.State.CLOSE)); - /** - * Try the same and see if this also works if node was just restarted. + /* Try the same and see if this also works if node was just restarted. * Each node holds an array of indices it knows of and checks if it should * write new meta data by looking up in this array. We need it because if an * index is closed it will not appear in the shard routing and we therefore * need to keep track of what we wrote before. However, when the node is * restarted this array is empty and we have to fill it before we decide - * what we write. This is why I explicitly test for it. + * what we write. This is why we explicitly test for it. */ internalCluster().restartNode(dataNode, new RestartCallback()); client().admin().indices().preparePutMapping(index).setType("doc").setSource(jsonBuilder().startObject() @@ -151,19 +150,9 @@ public class MetaDataWriteDataNodesTests extends ElasticsearchIntegrationTest { assertThat(indicesMetaData.get(index).state(), equalTo(IndexMetaData.State.CLOSE)); // finally check that meta data is also written of index opened again - client().admin().indices().prepareOpen(index).get(); - assertBusy(new Runnable() { - @Override - public void run() { - try { - ImmutableOpenMap indicesMetaData = getIndicesMetaDataOnNode(dataNode); - assertThat(indicesMetaData.get(index).state(), equalTo(IndexMetaData.State.OPEN)); - } catch (Exception e) { - logger.info("caught exception while reading meta state: ", e); - fail(); - } - } - }); + assertAcked(client().admin().indices().prepareOpen(index).get()); + indicesMetaData = getIndicesMetaDataOnNode(dataNode); + assertThat(indicesMetaData.get(index).state(), equalTo(IndexMetaData.State.OPEN)); } protected void assertIndexNotInMetaState(String nodeName, String indexName) throws Exception {