From a7947b404b3f32df9ebdaa35da836bd4bc0fa760 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 24 Apr 2017 09:06:36 -0400 Subject: [PATCH 1/8] Fix hash code for AliasFilter This commit fixes the hash code for AliasFilter as the previous implementation was neglecting to take into consideration the fact that the aliases field is an array and thus a deep hash code of it should be computed rather than a shallow hash code on the reference. Relates #24286 --- .../search/internal/AliasFilter.java | 2 +- .../search/internal/AliasFilterTests.java | 62 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/org/elasticsearch/search/internal/AliasFilterTests.java diff --git a/core/src/main/java/org/elasticsearch/search/internal/AliasFilter.java b/core/src/main/java/org/elasticsearch/search/internal/AliasFilter.java index 915e745ddf4..f053b3d48fe 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/AliasFilter.java +++ b/core/src/main/java/org/elasticsearch/search/internal/AliasFilter.java @@ -126,7 +126,7 @@ public final class AliasFilter implements Writeable { @Override public int hashCode() { - return Objects.hash(aliases, filter, reparseAliases); + return Objects.hash(reparseAliases, Arrays.hashCode(aliases), filter); } @Override diff --git a/core/src/test/java/org/elasticsearch/search/internal/AliasFilterTests.java b/core/src/test/java/org/elasticsearch/search/internal/AliasFilterTests.java new file mode 100644 index 00000000000..ba21a63f683 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/internal/AliasFilterTests.java @@ -0,0 +1,62 @@ +/* + * 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.search.internal; + +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.query.TermQueryBuilder; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.EqualsHashCodeTestUtils; + +import java.util.Arrays; + +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.instanceOf; + +public class AliasFilterTests extends ESTestCase { + + public void testEqualsAndHashCode() { + final QueryBuilder filter = QueryBuilders.termQuery("field", "value"); + final String[] aliases = new String[] { "alias_0", "alias_1" }; + final AliasFilter aliasFilter = new AliasFilter(filter, aliases); + final EqualsHashCodeTestUtils.CopyFunction aliasFilterCopyFunction = x -> { + assertThat(x.getQueryBuilder(), instanceOf(TermQueryBuilder.class)); + final BytesStreamOutput out = new BytesStreamOutput(); + x.getQueryBuilder().writeTo(out); + final QueryBuilder otherFilter = new TermQueryBuilder(out.bytes().streamInput()); + final String[] otherAliases = Arrays.copyOf(x.getAliases(), x.getAliases().length); + return new AliasFilter(otherFilter, otherAliases); + }; + + final EqualsHashCodeTestUtils.MutateFunction aliasFilterMutationFunction = x -> { + assertThat(x.getQueryBuilder(), instanceOf(TermQueryBuilder.class)); + final BytesStreamOutput out = new BytesStreamOutput(); + x.getQueryBuilder().writeTo(out); + final QueryBuilder otherFilter = new TermQueryBuilder(out.bytes().streamInput()); + assertThat(x.getAliases().length, greaterThan(0)); + final String[] otherAliases = Arrays.copyOf(x.getAliases(), x.getAliases().length - 1); + return new AliasFilter(otherFilter, otherAliases); + }; + + EqualsHashCodeTestUtils.checkEqualsAndHashCode(aliasFilter, aliasFilterCopyFunction, aliasFilterMutationFunction); + } + +} From 1500beafc7baad4e8cbce0682ed76e429498ea13 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 24 Apr 2017 09:31:54 -0400 Subject: [PATCH 2/8] Check for default.path.data included in path.data If the user explicitly configured path.data to include default.path.data, then we should not fail the node if we find indices in default.path.data. This commit addresses this. Relates #24285 --- .../java/org/elasticsearch/node/Node.java | 21 ++++- .../org/elasticsearch/node/NodeTests.java | 0 .../org/elasticsearch/node/EvilNodeTests.java | 77 +++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) rename {test/framework/src/main => core/src/test}/java/org/elasticsearch/node/NodeTests.java (100%) create mode 100644 qa/evil-tests/src/test/java/org/elasticsearch/node/EvilNodeTests.java diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 2508872eed1..103add62809 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -523,15 +523,21 @@ public class Node implements Closeable { boolean clean = true; for (final String defaultPathData : Environment.DEFAULT_PATH_DATA_SETTING.get(settings)) { - final Path nodeDirectory = NodeEnvironment.resolveNodePath(getPath(defaultPathData), nodeEnv.getNodeLockId()); - if (Files.exists(nodeDirectory) == false) { + final Path defaultNodeDirectory = NodeEnvironment.resolveNodePath(getPath(defaultPathData), nodeEnv.getNodeLockId()); + if (Files.exists(defaultNodeDirectory) == false) { continue; } - final NodeEnvironment.NodePath nodePath = new NodeEnvironment.NodePath(nodeDirectory); + + if (isDefaultPathDataInPathData(nodeEnv, defaultNodeDirectory)) { + continue; + } + + final NodeEnvironment.NodePath nodePath = new NodeEnvironment.NodePath(defaultNodeDirectory); final Set availableIndexFolders = nodeEnv.availableIndexFoldersForPath(nodePath); if (availableIndexFolders.isEmpty()) { continue; } + clean = false; logger.error("detected index data in default.path.data [{}] where there should not be any", nodePath.indicesPath); for (final String availableIndexFolder : availableIndexFolders) { @@ -554,6 +560,15 @@ public class Node implements Closeable { throw new IllegalStateException(message); } + private static boolean isDefaultPathDataInPathData(final NodeEnvironment nodeEnv, final Path defaultNodeDirectory) throws IOException { + for (final NodeEnvironment.NodePath dataPath : nodeEnv.nodePaths()) { + if (Files.isSameFile(dataPath.path, defaultNodeDirectory)) { + return true; + } + } + return false; + } + @SuppressForbidden(reason = "read path that is not configured in environment") private static Path getPath(final String path) { return PathUtils.get(path); diff --git a/test/framework/src/main/java/org/elasticsearch/node/NodeTests.java b/core/src/test/java/org/elasticsearch/node/NodeTests.java similarity index 100% rename from test/framework/src/main/java/org/elasticsearch/node/NodeTests.java rename to core/src/test/java/org/elasticsearch/node/NodeTests.java diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/node/EvilNodeTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/node/EvilNodeTests.java new file mode 100644 index 00000000000..ac29902627d --- /dev/null +++ b/qa/evil-tests/src/test/java/org/elasticsearch/node/EvilNodeTests.java @@ -0,0 +1,77 @@ +/* + * 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.node; + +import org.apache.logging.log4j.Logger; +import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; +import org.elasticsearch.env.NodeEnvironment; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +public class EvilNodeTests extends ESTestCase { + + public void testDefaultPathDataIncludedInPathData() throws IOException { + final Path zero = createTempDir().toAbsolutePath(); + final Path one = createTempDir().toAbsolutePath(); + final int random = randomIntBetween(0, 2); + final Path defaultPathData; + final Path choice = randomFrom(zero, one); + switch (random) { + case 0: + defaultPathData = choice; + break; + case 1: + defaultPathData = createTempDir().toAbsolutePath().resolve("link"); + Files.createSymbolicLink(defaultPathData, choice); + break; + case 2: + defaultPathData = createTempDir().toAbsolutePath().resolve("link"); + Files.createLink(defaultPathData, choice); + break; + default: + throw new AssertionError(Integer.toString(random)); + } + final Settings settings = Settings.builder() + .put("path.home", createTempDir().toAbsolutePath()) + .put("path.data.0", zero) + .put("path.data.1", one) + .put("default.path.data", defaultPathData) + .build(); + try (NodeEnvironment nodeEnv = new NodeEnvironment(settings, new Environment(settings))) { + final Path defaultPathDataWithNodesAndId = defaultPathData.resolve("nodes/0"); + Files.createDirectories(defaultPathDataWithNodesAndId); + final NodeEnvironment.NodePath defaultNodePath = new NodeEnvironment.NodePath(defaultPathDataWithNodesAndId); + Files.createDirectories(defaultNodePath.indicesPath.resolve(UUIDs.randomBase64UUID())); + final Logger mock = mock(Logger.class); + // nothing should happen here + Node.checkForIndexDataInDefaultPathData(settings, nodeEnv, mock); + verifyNoMoreInteractions(mock); + } + } + +} From 931198688c2d34555fb7d82a0d2cf34feea70f1d Mon Sep 17 00:00:00 2001 From: farisk Date: Mon, 24 Apr 2017 15:28:46 +0100 Subject: [PATCH 3/8] Document that painless doesn't support the "advanced text scoring" (#24271) I just spent ages debugging a script I wrote after following the documentation. It was not clear to me that _index is not defined when using painless; if it was mentioned on this page I would have saved myself a lot of time. --- docs/reference/modules/scripting/advanced-scripting.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/reference/modules/scripting/advanced-scripting.asciidoc b/docs/reference/modules/scripting/advanced-scripting.asciidoc index b4053d76598..a5fcc12777d 100644 --- a/docs/reference/modules/scripting/advanced-scripting.asciidoc +++ b/docs/reference/modules/scripting/advanced-scripting.asciidoc @@ -10,6 +10,8 @@ script inside a <>. Statistics over the document collection are computed *per shard*, not per index. +It should be noted that the `_index` variable is not supported in the painless language, but `_index` is defined when using the groovy language. + [float] === Nomenclature: From 373edee29a676444c0f423b9ee202fb0735d3ccd Mon Sep 17 00:00:00 2001 From: Nilabh Sagar Date: Mon, 24 Apr 2017 20:05:14 +0530 Subject: [PATCH 4/8] Provide informative error message in case of unknown suggestion context. (#24241) Provide a list of available contexts when you send an unknown context to the completion suggester. --- .../completion/context/ContextMappings.java | 4 ++- .../CategoryContextMappingTests.java | 26 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java index 0cac17dbb81..bbefbf41b89 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java @@ -81,7 +81,9 @@ public class ContextMappings implements ToXContent { public ContextMapping get(String name) { ContextMapping contextMapping = contextNameMap.get(name); if (contextMapping == null) { - throw new IllegalArgumentException("Unknown context name[" + name + "], must be one of " + contextNameMap.size()); + List keys = new ArrayList<>(contextNameMap.keySet()); + Collections.sort(keys); + throw new IllegalArgumentException("Unknown context name [" + name + "], must be one of " + keys.toString()); } return contextMapping; } diff --git a/core/src/test/java/org/elasticsearch/search/suggest/completion/CategoryContextMappingTests.java b/core/src/test/java/org/elasticsearch/search/suggest/completion/CategoryContextMappingTests.java index a9639c7b465..b9b15341da0 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/completion/CategoryContextMappingTests.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/completion/CategoryContextMappingTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.index.mapper.CompletionFieldMapper.CompletionFieldType; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; @@ -673,6 +674,31 @@ public class CategoryContextMappingTests extends ESSingleNodeTestCase { Exception e = expectThrows(ElasticsearchParseException.class, () -> mapping.parseQueryContext(createParseContext(parser))); assertEquals("category context must be an object, string, number or boolean", e.getMessage()); } + + public void testUnknownQueryContextParsing() throws Exception { + String mapping = jsonBuilder().startObject().startObject("type1") + .startObject("properties").startObject("completion") + .field("type", "completion") + .startArray("contexts") + .startObject() + .field("name", "ctx") + .field("type", "category") + .endObject() + .startObject() + .field("name", "type") + .field("type", "category") + .endObject() + .endArray() + .endObject().endObject() + .endObject().endObject().string(); + + DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse("type1", new CompressedXContent(mapping)); + FieldMapper fieldMapper = defaultMapper.mappers().getMapper("completion"); + CompletionFieldType completionFieldType = (CompletionFieldType) fieldMapper.fieldType(); + + Exception e = expectThrows(IllegalArgumentException.class, () -> completionFieldType.getContextMappings().get("brand")); + assertEquals("Unknown context name [brand], must be one of [ctx, type]", e.getMessage()); + } public void testParsingContextFromDocument() throws Exception { CategoryContextMapping mapping = ContextBuilder.category("cat").field("category").build(); From dabbf5d4f44c6bd98d1fad1ca71edfb15ca4b94f Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 19 Apr 2017 10:23:27 +0200 Subject: [PATCH 5/8] [TEST] Added unittests for InternalGeoCentroid Relates to #22278 --- .../geocentroid/InternalGeoCentroid.java | 21 ++++++ .../geocentroid/InternalGeoCentroidTests.java | 66 +++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 core/src/test/java/org/elasticsearch/search/aggregations/metrics/geocentroid/InternalGeoCentroidTests.java diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/geocentroid/InternalGeoCentroid.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/geocentroid/InternalGeoCentroid.java index bd65cd28aff..da69115ac6b 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/geocentroid/InternalGeoCentroid.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/geocentroid/InternalGeoCentroid.java @@ -30,6 +30,7 @@ import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.Objects; /** * Serialization and merge logic for {@link GeoCentroidAggregator}. @@ -154,4 +155,24 @@ public class InternalGeoCentroid extends InternalAggregation implements GeoCentr } return builder; } + + @Override + public boolean doEquals(Object o) { + InternalGeoCentroid that = (InternalGeoCentroid) o; + return count == that.count && + Objects.equals(centroid, that.centroid); + } + + @Override + protected int doHashCode() { + return Objects.hash(centroid, count); + } + + @Override + public String toString() { + return "InternalGeoCentroid{" + + "centroid=" + centroid + + ", count=" + count + + '}'; + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/geocentroid/InternalGeoCentroidTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/geocentroid/InternalGeoCentroidTests.java new file mode 100644 index 00000000000..c409d2aa795 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/geocentroid/InternalGeoCentroidTests.java @@ -0,0 +1,66 @@ +/* + * 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.search.aggregations.metrics.geocentroid; + +import org.apache.lucene.geo.GeoEncodingUtils; +import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.search.aggregations.InternalAggregationTestCase; +import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; +import org.elasticsearch.test.geo.RandomGeoGenerator; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +public class InternalGeoCentroidTests extends InternalAggregationTestCase { + + @Override + protected InternalGeoCentroid createTestInstance(String name, List pipelineAggregators, + Map metaData) { + GeoPoint centroid = RandomGeoGenerator.randomPoint(random()); + + // Re-encode lat/longs to avoid rounding issue when testing InternalGeoCentroid#hashCode() and + // InternalGeoCentroid#equals() + int encodedLon = GeoEncodingUtils.encodeLongitude(centroid.lon()); + centroid.resetLon(GeoEncodingUtils.decodeLongitude(encodedLon)); + int encodedLat = GeoEncodingUtils.encodeLatitude(centroid.lat()); + centroid.resetLat(GeoEncodingUtils.decodeLatitude(encodedLat)); + + return new InternalGeoCentroid("_name", centroid, 1, Collections.emptyList(), Collections.emptyMap()); + } + + @Override + protected Writeable.Reader instanceReader() { + return InternalGeoCentroid::new; + } + + @Override + protected void assertReduced(InternalGeoCentroid reduced, List inputs) { + GeoPoint expected = new GeoPoint(0, 0); + int i = 0; + for (InternalGeoCentroid input : inputs) { + expected.reset(expected.lat() + (input.centroid().lat() - expected.lat()) / (i+1), + expected.lon() + (input.centroid().lon() - expected.lon()) / (i+1)); + i++; + } + assertEquals(expected.getLat(), reduced.centroid().getLat(), 1E-5D); + assertEquals(expected.getLon(), reduced.centroid().getLon(), 1E-5D); + } +} From c5b6f52eccb2a663770e0eb9fee496432e1a8d5c Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Mon, 24 Apr 2017 10:59:08 -0400 Subject: [PATCH 6/8] Fixes maintaining the shards a snapshot is waiting on (#24289) There was a bug in the calculation of the shards that a snapshot must wait on, due to their relocating or initializing, before the snapshot can proceed safely to snapshot the shard data. In this bug, an incorrect key was used to look up the index of the waiting shards, resulting in the fact that each index would have at most one shard in the waiting state causing the snapshot to pause. This could be problematic if there are more than one shard in the relocating or initializing state, which would result in a snapshot prematurely starting because it thinks its only waiting on one relocating or initializing shard (when in fact there could be more than one). While not a common case and likely rare in practice, it is still problematic. This commit fixes the issue by ensuring the correct key is used to look up the waiting indices map as it is being built up, so the list of waiting shards for each index (those shards that are relocating or initializing) are aggregated for a given index instead of overwritten. --- .../cluster/SnapshotsInProgress.java | 9 ++- .../cluster/SnapshotsInProgressTests.java | 78 +++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/cluster/SnapshotsInProgressTests.java diff --git a/core/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java b/core/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java index 1e1b61281b4..5b8d3918894 100644 --- a/core/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java +++ b/core/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java @@ -195,14 +195,16 @@ public class SnapshotsInProgress extends AbstractNamedDiffable implement return snapshot.toString(); } - private ImmutableOpenMap> findWaitingIndices(ImmutableOpenMap shards) { + // package private for testing + ImmutableOpenMap> findWaitingIndices(ImmutableOpenMap shards) { Map> waitingIndicesMap = new HashMap<>(); for (ObjectObjectCursor entry : shards) { if (entry.value.state() == State.WAITING) { - List waitingShards = waitingIndicesMap.get(entry.key.getIndex()); + final String indexName = entry.key.getIndexName(); + List waitingShards = waitingIndicesMap.get(indexName); if (waitingShards == null) { waitingShards = new ArrayList<>(); - waitingIndicesMap.put(entry.key.getIndexName(), waitingShards); + waitingIndicesMap.put(indexName, waitingShards); } waitingShards.add(entry.key); } @@ -216,7 +218,6 @@ public class SnapshotsInProgress extends AbstractNamedDiffable implement } return waitingIndicesBuilder.build(); } - } /** diff --git a/core/src/test/java/org/elasticsearch/cluster/SnapshotsInProgressTests.java b/core/src/test/java/org/elasticsearch/cluster/SnapshotsInProgressTests.java new file mode 100644 index 00000000000..4d1a1a6e588 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/cluster/SnapshotsInProgressTests.java @@ -0,0 +1,78 @@ +/* + * 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; + +import org.elasticsearch.cluster.SnapshotsInProgress.Entry; +import org.elasticsearch.cluster.SnapshotsInProgress.ShardSnapshotStatus; +import org.elasticsearch.cluster.SnapshotsInProgress.State; +import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.repositories.IndexId; +import org.elasticsearch.snapshots.Snapshot; +import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.test.ESTestCase; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Unit tests for the {@link SnapshotsInProgress} class and its inner classes. + */ +public class SnapshotsInProgressTests extends ESTestCase { + + /** + * Makes sure that the indices being waited on before snapshotting commences + * are populated with all shards in the relocating or initializing state. + */ + public void testWaitingIndices() { + final Snapshot snapshot = new Snapshot("repo", new SnapshotId("snap", randomAlphaOfLength(5))); + final String idx1Name = "idx1"; + final String idx2Name = "idx2"; + final String idx3Name = "idx3"; + final String idx1UUID = randomAlphaOfLength(5); + final String idx2UUID = randomAlphaOfLength(5); + final String idx3UUID = randomAlphaOfLength(5); + final List indices = Arrays.asList(new IndexId(idx1Name, randomAlphaOfLength(5)), + new IndexId(idx2Name, randomAlphaOfLength(5)), new IndexId(idx3Name, randomAlphaOfLength(5))); + ImmutableOpenMap.Builder shards = ImmutableOpenMap.builder(); + + // test more than one waiting shard in an index + shards.put(new ShardId(idx1Name, idx1UUID, 0), new ShardSnapshotStatus(randomAlphaOfLength(2), State.WAITING)); + shards.put(new ShardId(idx1Name, idx1UUID, 1), new ShardSnapshotStatus(randomAlphaOfLength(2), State.WAITING)); + shards.put(new ShardId(idx1Name, idx1UUID, 2), new ShardSnapshotStatus(randomAlphaOfLength(2), randomNonWaitingState())); + // test exactly one waiting shard in an index + shards.put(new ShardId(idx2Name, idx2UUID, 0), new ShardSnapshotStatus(randomAlphaOfLength(2), State.WAITING)); + shards.put(new ShardId(idx2Name, idx2UUID, 1), new ShardSnapshotStatus(randomAlphaOfLength(2), randomNonWaitingState())); + // test no waiting shards in an index + shards.put(new ShardId(idx3Name, idx3UUID, 0), new ShardSnapshotStatus(randomAlphaOfLength(2), randomNonWaitingState())); + Entry entry = new Entry(snapshot, randomBoolean(), randomBoolean(), State.INIT, + indices, System.currentTimeMillis(), randomLong(), shards.build()); + + ImmutableOpenMap> waitingIndices = entry.waitingIndices(); + assertEquals(2, waitingIndices.get(idx1Name).size()); + assertEquals(1, waitingIndices.get(idx2Name).size()); + assertFalse(waitingIndices.containsKey(idx3Name)); + } + + private State randomNonWaitingState() { + return randomFrom(Arrays.stream(State.values()).filter(s -> s != State.WAITING).collect(Collectors.toSet())); + } +} From c9a6d66bd5f67e9fc65204529234a9f3ee3b2932 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 24 Apr 2017 11:02:21 -0400 Subject: [PATCH 7/8] Only test hard linking to directory on macOS This skips trying to create a hard link to a directory in the evil node tests on non-macOS operating systems. --- .../test/java/org/elasticsearch/node/EvilNodeTests.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/node/EvilNodeTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/node/EvilNodeTests.java index ac29902627d..341d1227926 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/node/EvilNodeTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/node/EvilNodeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.node; import org.apache.logging.log4j.Logger; +import org.apache.lucene.util.Constants; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; @@ -38,7 +39,13 @@ public class EvilNodeTests extends ESTestCase { public void testDefaultPathDataIncludedInPathData() throws IOException { final Path zero = createTempDir().toAbsolutePath(); final Path one = createTempDir().toAbsolutePath(); - final int random = randomIntBetween(0, 2); + // creating hard links to directories is okay on macOS so we exercise it here + final int random; + if (Constants.MAC_OS_X) { + random = randomFrom(0, 1, 2); + } else { + random = randomFrom(0, 1); + } final Path defaultPathData; final Path choice = randomFrom(zero, one); switch (random) { From 026bf2e3ee1ee5c188e57839d9e902962158138b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 24 Apr 2017 18:40:57 +0200 Subject: [PATCH 8/8] Remove getCountAsString() from InternalStats and Stats interface (#24291) The `count` value in the stats aggregation represents a simple doc count that doesn't require a formatted version. We didn't render an "as_string" version for count in the rest response, so the method should also be removed in favour of just using String.valueOf(getCount()) if a string version of the count is needed. Closes #24287 --- .../search/aggregations/metrics/stats/InternalStats.java | 5 ----- .../search/aggregations/metrics/stats/Stats.java | 5 ----- .../search/aggregations/metrics/ExtendedStatsIT.java | 1 - .../elasticsearch/search/aggregations/metrics/StatsIT.java | 1 - docs/reference/migration/migrate_6_0/java.asciidoc | 6 ++++++ 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java index 08c9292d54e..b0b2ea73d3c 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java @@ -112,11 +112,6 @@ public class InternalStats extends InternalNumericMetricsAggregation.MultiValue return sum; } - @Override - public String getCountAsString() { - return valueAsString(Metrics.count.name()); - } - @Override public String getMinAsString() { return valueAsString(Metrics.min.name()); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/Stats.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/Stats.java index 4910dc14002..46620f51dc2 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/Stats.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/Stats.java @@ -50,11 +50,6 @@ public interface Stats extends NumericMetricsAggregation.MultiValue { */ double getSum(); - /** - * @return The number of values that were aggregated as a String. - */ - String getCountAsString(); - /** * @return The minimum value of all aggregated values as a String. */ diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java index 6ec3f090d45..7de7bb0f315 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java @@ -214,7 +214,6 @@ public class ExtendedStatsIT extends AbstractNumericTestCase { assertThat(stats.getSum(), equalTo((double) 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10)); assertThat(stats.getSumAsString(), equalTo("0055.0")); assertThat(stats.getCount(), equalTo(10L)); - assertThat(stats.getCountAsString(), equalTo("0010.0")); assertThat(stats.getSumOfSquares(), equalTo((double) 1 + 4 + 9 + 16 + 25 + 36 + 49 + 64 + 81 + 100)); assertThat(stats.getSumOfSquaresAsString(), equalTo("0385.0")); assertThat(stats.getVariance(), equalTo(variance(1, 2, 3, 4, 5, 6, 7, 8, 9, 10))); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java index 8b849f625c1..85db6166307 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java @@ -162,7 +162,6 @@ public class StatsIT extends AbstractNumericTestCase { assertThat(stats.getSum(), equalTo((double) 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10)); assertThat(stats.getSumAsString(), equalTo("0055.0")); assertThat(stats.getCount(), equalTo(10L)); - assertThat(stats.getCountAsString(), equalTo("0010.0")); } @Override diff --git a/docs/reference/migration/migrate_6_0/java.asciidoc b/docs/reference/migration/migrate_6_0/java.asciidoc index 991fe165fb2..a8954732657 100644 --- a/docs/reference/migration/migrate_6_0/java.asciidoc +++ b/docs/reference/migration/migrate_6_0/java.asciidoc @@ -13,3 +13,9 @@ providing the source in bytes or as a string. In previous versions of Elasticsearch, delete by query requests without an explicit query were accepted, match_all was used as the default query and all documents were deleted as a result. From version 6.0.0, a `DeleteByQueryRequest` requires an explicit query be set. + +=== `InternalStats` and `Stats` getCountAsString() method removed + +The `count` value in the stats aggregation represents a doc count that shouldnn't require a formatted +version. This method was deprecated in 5.4 in favour of just using +`String.valueOf(getCount())` if needed