From f763037b03d9a9d76575bb94efbe40b31496203e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 3 Dec 2018 11:11:10 +0100 Subject: [PATCH 01/17] MINOR: BlobstoreRepository Cleanups (#36140) * Removed redundant private getter * Removed unused `version` field --- .../blobstore/BlobStoreRepository.java | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 75026780b9aa3..b458b831d73b4 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -458,7 +458,7 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) { if (indexMetaData != null) { for (int shardId = 0; shardId < indexMetaData.getNumberOfShards(); shardId++) { try { - delete(snapshotId, snapshot.version(), indexId, new ShardId(indexMetaData.getIndex(), shardId)); + delete(snapshotId, indexId, new ShardId(indexMetaData.getIndex(), shardId)); } catch (SnapshotException ex) { final int finalShardId = shardId; logger.warn(() -> new ParameterizedMessage("[{}] failed to delete shard data for shard [{}][{}]", @@ -864,7 +864,7 @@ public void snapshotShard(IndexShard shard, Store store, SnapshotId snapshotId, @Override public void restoreShard(IndexShard shard, SnapshotId snapshotId, Version version, IndexId indexId, ShardId snapshotShardId, RecoveryState recoveryState) { - final RestoreContext snapshotContext = new RestoreContext(shard, snapshotId, version, indexId, snapshotShardId, recoveryState); + final RestoreContext snapshotContext = new RestoreContext(shard, snapshotId, indexId, snapshotShardId, recoveryState); try { snapshotContext.restore(); } catch (Exception e) { @@ -874,7 +874,7 @@ public void restoreShard(IndexShard shard, SnapshotId snapshotId, Version versio @Override public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, Version version, IndexId indexId, ShardId shardId) { - Context context = new Context(snapshotId, version, indexId, shardId); + Context context = new Context(snapshotId, indexId, shardId); BlobStoreIndexShardSnapshot snapshot = context.loadSnapshot(); return IndexShardSnapshotStatus.newDone(snapshot.startTime(), snapshot.time(), snapshot.incrementalFileCount(), snapshot.totalFileCount(), @@ -916,8 +916,8 @@ public void verify(String seed, DiscoveryNode localNode) { * @param snapshotId snapshot id * @param shardId shard id */ - private void delete(SnapshotId snapshotId, Version version, IndexId indexId, ShardId shardId) { - Context context = new Context(snapshotId, version, indexId, shardId, shardId); + private void delete(SnapshotId snapshotId, IndexId indexId, ShardId shardId) { + Context context = new Context(snapshotId, indexId, shardId, shardId); context.delete(); } @@ -929,10 +929,6 @@ public String toString() { ']'; } - BlobStoreFormat indexShardSnapshotFormat(Version version) { - return indexShardSnapshotFormat; - } - /** * Context for snapshot/restore operations */ @@ -944,15 +940,12 @@ private class Context { protected final BlobContainer blobContainer; - protected final Version version; - - Context(SnapshotId snapshotId, Version version, IndexId indexId, ShardId shardId) { - this(snapshotId, version, indexId, shardId, shardId); + Context(SnapshotId snapshotId, IndexId indexId, ShardId shardId) { + this(snapshotId, indexId, shardId, shardId); } - Context(SnapshotId snapshotId, Version version, IndexId indexId, ShardId shardId, ShardId snapshotShardId) { + Context(SnapshotId snapshotId, IndexId indexId, ShardId shardId, ShardId snapshotShardId) { this.snapshotId = snapshotId; - this.version = version; this.shardId = shardId; blobContainer = blobStore().blobContainer(basePath().add("indices").add(indexId.getId()).add(Integer.toString(snapshotShardId.getId()))); } @@ -973,7 +966,7 @@ public void delete() { int fileListGeneration = tuple.v2(); try { - indexShardSnapshotFormat(version).delete(blobContainer, snapshotId.getUUID()); + indexShardSnapshotFormat.delete(blobContainer, snapshotId.getUUID()); } catch (IOException e) { logger.debug("[{}] [{}] failed to delete shard snapshot file", shardId, snapshotId); } @@ -994,7 +987,7 @@ public void delete() { */ BlobStoreIndexShardSnapshot loadSnapshot() { try { - return indexShardSnapshotFormat(version).read(blobContainer, snapshotId.getUUID()); + return indexShardSnapshotFormat.read(blobContainer, snapshotId.getUUID()); } catch (IOException ex) { throw new SnapshotException(metadata.name(), snapshotId, "failed to read shard snapshot file for " + shardId, ex); } @@ -1175,7 +1168,7 @@ private class SnapshotContext extends Context { * @param snapshotStatus snapshot status to report progress */ SnapshotContext(Store store, SnapshotId snapshotId, IndexId indexId, IndexShardSnapshotStatus snapshotStatus, long startTime) { - super(snapshotId, Version.CURRENT, indexId, store.shardId()); + super(snapshotId, indexId, store.shardId()); this.snapshotStatus = snapshotStatus; this.store = store; this.startTime = startTime; @@ -1316,7 +1309,6 @@ public void snapshot(final IndexCommit snapshotIndexCommit) { // finalize the snapshot and rewrite the snapshot index with the next sequential snapshot index finalize(newSnapshotsList, fileListGeneration + 1, blobs, "snapshot creation [" + snapshotId + "]"); snapshotStatus.moveToDone(System.currentTimeMillis()); - } /** @@ -1476,8 +1468,8 @@ private class RestoreContext extends Context { * @param snapshotShardId shard in the snapshot that data should be restored from * @param recoveryState recovery state to report progress */ - RestoreContext(IndexShard shard, SnapshotId snapshotId, Version version, IndexId indexId, ShardId snapshotShardId, RecoveryState recoveryState) { - super(snapshotId, version, indexId, shard.shardId(), snapshotShardId); + RestoreContext(IndexShard shard, SnapshotId snapshotId, IndexId indexId, ShardId snapshotShardId, RecoveryState recoveryState) { + super(snapshotId, indexId, shard.shardId(), snapshotShardId); this.recoveryState = recoveryState; this.targetShard = shard; } From 328d022ddd8b14e31a8e89dd0becd22136ed04cb Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 3 Dec 2018 11:21:42 +0100 Subject: [PATCH 02/17] MINOR: Some Cleanups around Store (#36139) * Moved method `canOpenIndex` is only used in tests -> moved to test CP * Simplify `org.elasticsearch.index.store.Store#renameTempFilesSafe` * Delete some dead methods --- .../org/elasticsearch/index/store/Store.java | 60 ++++--------------- .../index/shard/IndexShardTests.java | 3 +- .../index/shard/ShardUtilsTests.java | 5 -- .../elasticsearch/index/store/StoreTests.java | 6 +- .../elasticsearch/index/store/StoreUtils.java | 46 ++++++++++++++ 5 files changed, 64 insertions(+), 56 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/index/store/StoreUtils.java diff --git a/server/src/main/java/org/elasticsearch/index/store/Store.java b/server/src/main/java/org/elasticsearch/index/store/Store.java index c1c3b86708b74..3bec89549a780 100644 --- a/server/src/main/java/org/elasticsearch/index/store/Store.java +++ b/server/src/main/java/org/elasticsearch/index/store/Store.java @@ -91,7 +91,6 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -300,21 +299,18 @@ public MetadataSnapshot getMetadata(IndexCommit commit, boolean lockDirectory) t public void renameTempFilesSafe(Map tempFileMap) throws IOException { // this works just like a lucene commit - we rename all temp files and once we successfully // renamed all the segments we rename the commit to ensure we don't leave half baked commits behind. - final Map.Entry[] entries = tempFileMap.entrySet().toArray(new Map.Entry[tempFileMap.size()]); - ArrayUtil.timSort(entries, new Comparator>() { - @Override - public int compare(Map.Entry o1, Map.Entry o2) { - String left = o1.getValue(); - String right = o2.getValue(); - if (left.startsWith(IndexFileNames.SEGMENTS) || right.startsWith(IndexFileNames.SEGMENTS)) { - if (left.startsWith(IndexFileNames.SEGMENTS) == false) { - return -1; - } else if (right.startsWith(IndexFileNames.SEGMENTS) == false) { - return 1; - } + final Map.Entry[] entries = tempFileMap.entrySet().toArray(new Map.Entry[0]); + ArrayUtil.timSort(entries, (o1, o2) -> { + String left = o1.getValue(); + String right = o2.getValue(); + if (left.startsWith(IndexFileNames.SEGMENTS) || right.startsWith(IndexFileNames.SEGMENTS)) { + if (left.startsWith(IndexFileNames.SEGMENTS) == false) { + return -1; + } else if (right.startsWith(IndexFileNames.SEGMENTS) == false) { + return 1; } - return left.compareTo(right); } + return left.compareTo(right); }); metadataLock.writeLock().lock(); // we make sure that nobody fetches the metadata while we do this rename operation here to ensure we don't @@ -454,22 +450,6 @@ public static MetadataSnapshot readMetadataSnapshot(Path indexLocation, ShardId return MetadataSnapshot.EMPTY; } - /** - * Returns true iff the given location contains an index an the index - * can be successfully opened. This includes reading the segment infos and possible - * corruption markers. - */ - public static boolean canOpenIndex(Logger logger, Path indexLocation, - ShardId shardId, NodeEnvironment.ShardLocker shardLocker) throws IOException { - try { - tryOpenIndex(indexLocation, shardId, shardLocker, logger); - } catch (Exception ex) { - logger.trace(() -> new ParameterizedMessage("Can't open index for path [{}]", indexLocation), ex); - return false; - } - return true; - } - /** * Tries to open an index for the given location. This includes reading the * segment infos and possible corruption markers. If the index can not @@ -961,7 +941,6 @@ public Map asMap() { private static final String DEL_FILE_EXTENSION = "del"; // legacy delete file private static final String LIV_FILE_EXTENSION = "liv"; // lucene 5 delete file - private static final String FIELD_INFOS_FILE_EXTENSION = "fnm"; private static final String SEGMENT_INFO_EXTENSION = "si"; /** @@ -1015,12 +994,7 @@ public RecoveryDiff recoveryDiff(MetadataSnapshot recoveryTargetSnapshot) { // only treat del files as per-commit files fnm files are generational but only for upgradable DV perCommitStoreFiles.add(meta); } else { - List perSegStoreFiles = perSegment.get(segmentId); - if (perSegStoreFiles == null) { - perSegStoreFiles = new ArrayList<>(); - perSegment.put(segmentId, perSegStoreFiles); - } - perSegStoreFiles.add(meta); + perSegment.computeIfAbsent(segmentId, k -> new ArrayList<>()).add(meta); } } final ArrayList identicalFiles = new ArrayList<>(); @@ -1072,13 +1046,6 @@ public String getHistoryUUID() { return commitUserData.get(Engine.HISTORY_UUID_KEY); } - /** - * returns the translog uuid the store points at - */ - public String getTranslogUUID() { - return commitUserData.get(Translog.TRANSLOG_UUID_KEY); - } - /** * Returns true iff this metadata contains the given file. */ @@ -1598,15 +1565,14 @@ public void trimUnsafeCommits(final long lastSyncedGlobalCheckpoint, final long } } - - private void updateCommitData(IndexWriter writer, Map keysToUpdate) throws IOException { + private static void updateCommitData(IndexWriter writer, Map keysToUpdate) throws IOException { final Map userData = getUserData(writer); userData.putAll(keysToUpdate); writer.setLiveCommitData(userData.entrySet()); writer.commit(); } - private Map getUserData(IndexWriter writer) { + private static Map getUserData(IndexWriter writer) { final Map userData = new HashMap<>(); writer.getLiveCommitData().forEach(e -> userData.put(e.getKey(), e.getValue())); return userData; diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 970e8ed5c9fab..0791ead6608a3 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -106,6 +106,7 @@ import org.elasticsearch.index.snapshots.IndexShardSnapshotStatus; import org.elasticsearch.index.store.Store; import org.elasticsearch.index.store.StoreStats; +import org.elasticsearch.index.store.StoreUtils; import org.elasticsearch.index.translog.TestTranslog; import org.elasticsearch.index.translog.Translog; import org.elasticsearch.index.translog.TranslogStats; @@ -261,7 +262,7 @@ public void testFailShard() throws Exception { ShardStateMetaData shardStateMetaData = load(logger, shardPath.getShardStatePath()); assertEquals(shardStateMetaData, getShardStateMetadata(shard)); // but index can't be opened for a failed shard - assertThat("store index should be corrupted", Store.canOpenIndex(logger, shardPath.resolveIndex(), shard.shardId(), + assertThat("store index should be corrupted", StoreUtils.canOpenIndex(logger, shardPath.resolveIndex(), shard.shardId(), (shardId, lockTimeoutMS) -> new DummyShardLock(shardId)), equalTo(false)); } diff --git a/server/src/test/java/org/elasticsearch/index/shard/ShardUtilsTests.java b/server/src/test/java/org/elasticsearch/index/shard/ShardUtilsTests.java index 7e9c7b901a212..8801f8bf5d260 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/ShardUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/ShardUtilsTests.java @@ -28,7 +28,6 @@ import org.apache.lucene.store.BaseDirectoryWrapper; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader; -import org.elasticsearch.index.engine.Engine; import org.elasticsearch.test.ESTestCase; import java.io.IOException; @@ -64,8 +63,4 @@ public void testExtractShardId() throws IOException { } IOUtils.close(writer, dir); } - - public static Engine getShardEngine(IndexShard shard) { - return shard.getEngine(); - } } diff --git a/server/src/test/java/org/elasticsearch/index/store/StoreTests.java b/server/src/test/java/org/elasticsearch/index/store/StoreTests.java index 7cf14bf84e331..82e0c88647e55 100644 --- a/server/src/test/java/org/elasticsearch/index/store/StoreTests.java +++ b/server/src/test/java/org/elasticsearch/index/store/StoreTests.java @@ -925,17 +925,17 @@ public void testCanOpenIndex() throws IOException { IndexWriterConfig iwc = newIndexWriterConfig(); Path tempDir = createTempDir(); final BaseDirectoryWrapper dir = newFSDirectory(tempDir); - assertFalse(Store.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id))); + assertFalse(StoreUtils.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id))); IndexWriter writer = new IndexWriter(dir, iwc); Document doc = new Document(); doc.add(new StringField("id", "1", random().nextBoolean() ? Field.Store.YES : Field.Store.NO)); writer.addDocument(doc); writer.commit(); writer.close(); - assertTrue(Store.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id))); + assertTrue(StoreUtils.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id))); Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId)); store.markStoreCorrupted(new CorruptIndexException("foo", "bar")); - assertFalse(Store.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id))); + assertFalse(StoreUtils.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id))); store.close(); } diff --git a/server/src/test/java/org/elasticsearch/index/store/StoreUtils.java b/server/src/test/java/org/elasticsearch/index/store/StoreUtils.java new file mode 100644 index 0000000000000..31732612e03ae --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/store/StoreUtils.java @@ -0,0 +1,46 @@ +/* + * 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.index.store; + +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.elasticsearch.env.NodeEnvironment; +import org.elasticsearch.index.shard.ShardId; + +import java.nio.file.Path; + +public final class StoreUtils { + + /** + * Returns {@code true} iff the given location contains an index an the index + * can be successfully opened. This includes reading the segment infos and possible + * corruption markers. + */ + public static boolean canOpenIndex(Logger logger, Path indexLocation, + ShardId shardId, NodeEnvironment.ShardLocker shardLocker) { + try { + Store.tryOpenIndex(indexLocation, shardId, shardLocker, logger); + } catch (Exception ex) { + logger.trace(() -> new ParameterizedMessage("Can't open index for path [{}]", indexLocation), ex); + return false; + } + return true; + } +} From 74aca756b80ff5ad5e633ab54f184cc3ec1ba7a0 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 3 Dec 2018 11:49:11 +0100 Subject: [PATCH 03/17] Remove the distinction between query and filter context in QueryBuilders (#35354) When building a query Lucene distinguishes two cases, queries that require to produce a score and queries that only need to match. We cloned this mechanism in the QueryBuilders in order to be able to produce different queries based on whether they need to produce a score or not. However the only case in es that require this distinction is the BoolQueryBuilder that sets a different minimum_should_match when a `bool` query is built in a filter context.. This behavior doesn't seem right because it makes the matching of `should` clauses different when the score is not required. Closes #35293 --- .../migration/migrate_7_0/search.asciidoc | 11 +++ docs/reference/query-dsl/bool-query.asciidoc | 17 +---- .../cluster/metadata/AliasValidator.java | 2 +- .../index/query/AbstractQueryBuilder.java | 13 ---- .../index/query/BoolQueryBuilder.java | 18 +---- .../query/ConstantScoreQueryBuilder.java | 2 +- .../index/query/FuzzyQueryBuilder.java | 3 - .../index/query/QueryBuilder.java | 10 --- .../index/query/QueryShardContext.java | 28 -------- .../index/query/SpanNearQueryBuilder.java | 5 -- .../search/DefaultSearchContext.java | 2 +- .../AdjacencyMatrixAggregatorFactory.java | 2 +- .../filter/FilterAggregatorFactory.java | 2 +- .../filter/FiltersAggregatorFactory.java | 2 +- .../SignificantTermsAggregatorFactory.java | 2 +- .../search/sort/SortBuilder.java | 4 +- .../index/query/BoolQueryBuilderTests.java | 45 ------------ .../query/SpanMultiTermQueryBuilderTests.java | 5 -- .../query/plugin/CustomQueryParserIT.java | 68 ------------------- .../index/query/plugin/DummyQueryBuilder.java | 4 +- .../query/plugin/DummyQueryParserPlugin.java | 5 -- .../bucket/filter/FilterAggregatorTests.java | 25 ------- .../bucket/filter/FiltersAggregatorTests.java | 24 ------- .../SignificantTermsAggregatorTests.java | 28 -------- .../aggregations/AggregatorTestCase.java | 3 - .../test/AbstractQueryTestCase.java | 6 -- .../SecurityIndexSearcherWrapper.java | 2 +- ...yIndexSearcherWrapperIntegrationTests.java | 2 +- 28 files changed, 26 insertions(+), 314 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc index 348763762aa5e..5774f17c3e3aa 100644 --- a/docs/reference/migration/migrate_7_0/search.asciidoc +++ b/docs/reference/migration/migrate_7_0/search.asciidoc @@ -159,3 +159,14 @@ Negative scores in the Function Score Query are deprecated in 6.x, and are not allowed in this version. If a negative score is produced as a result of computation (e.g. in `script_score` or `field_value_factor` functions), an error will be thrown. + +[float] +==== The filter context has been removed + +The `filter` context has been removed from Elasticsearch's query builders, +the distinction between queries and filters is now decided in Lucene depending +on whether queries need to access score or not. As a result `bool` queries with +`should` clauses that don't need to access the score will no longer set their +`minimum_should_match` to 1. This behavior has been deprecated in the previous +major version. + diff --git a/docs/reference/query-dsl/bool-query.asciidoc b/docs/reference/query-dsl/bool-query.asciidoc index a7092aaaab113..d4b5919454836 100644 --- a/docs/reference/query-dsl/bool-query.asciidoc +++ b/docs/reference/query-dsl/bool-query.asciidoc @@ -17,15 +17,7 @@ contribute to the score. in <>, meaning that scoring is ignored and clauses are considered for caching. -|`should` |The clause (query) should appear in the matching document. If the -`bool` query is in a <> and has a `must` or -`filter` clause then a document will match the `bool` query even if none of the -`should` queries match. In this case these clauses are only used to influence -the score. If the `bool` query is a <> -or has neither `must` or `filter` then at least one of the `should` queries -must match a document for it to match the `bool` query. This behavior may be -explicitly controlled by settings the -<> parameter. +|`should` |The clause (query) should appear in the matching document. |`must_not` |The clause (query) must not appear in the matching documents. Clauses are executed in <> meaning @@ -33,13 +25,6 @@ that scoring is ignored and clauses are considered for caching. Because scoring ignored, a score of `0` for all documents is returned. |======================================================================= -[IMPORTANT] -.Bool query in filter context -======================================================================== -If this query is used in a filter context and it has `should` -clauses then at least one `should` clause is required to match. -======================================================================== - The `bool` query takes a _more-matches-is-better_ approach, so the score from each matching `must` or `should` clause will be added together to provide the final `_score` for each document. diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java index c9258806d51db..ebcffacf0a6f5 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java @@ -139,6 +139,6 @@ public void validateAliasFilter(String alias, byte[] filter, QueryShardContext q private static void validateAliasFilter(XContentParser parser, QueryShardContext queryShardContext) throws IOException { QueryBuilder parseInnerQueryBuilder = parseInnerQueryBuilder(parser); QueryBuilder queryBuilder = Rewriteable.rewrite(parseInnerQueryBuilder, queryShardContext, true); - queryBuilder.toFilter(queryShardContext); + queryBuilder.toQuery(queryShardContext); } } diff --git a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index c0fb00128e556..cca1ca0fcc0d1 100644 --- a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -111,19 +111,6 @@ public final Query toQuery(QueryShardContext context) throws IOException { return query; } - @Override - public final Query toFilter(QueryShardContext context) throws IOException { - Query result; - final boolean originalIsFilter = context.isFilter(); - try { - context.setIsFilter(true); - result = toQuery(context); - } finally { - context.setIsFilter(originalIsFilter); - } - return result; - } - protected abstract Query doToQuery(QueryShardContext context) throws IOException; /** diff --git a/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index ee1779d3d5190..4c3c8944cf347 100644 --- a/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -384,12 +384,6 @@ protected Query doToQuery(QueryShardContext context) throws IOException { return new MatchAllDocsQuery(); } - final String minimumShouldMatch; - if (context.isFilter() && this.minimumShouldMatch == null && shouldClauses.size() > 0) { - minimumShouldMatch = "1"; - } else { - minimumShouldMatch = this.minimumShouldMatch; - } Query query = Queries.applyMinimumShouldMatch(booleanQuery, minimumShouldMatch); return adjustPureNegative ? fixNegativeQueryIfNeeded(query) : query; } @@ -397,17 +391,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException { private static void addBooleanClauses(QueryShardContext context, BooleanQuery.Builder booleanQueryBuilder, List clauses, Occur occurs) throws IOException { for (QueryBuilder query : clauses) { - Query luceneQuery = null; - switch (occurs) { - case MUST: - case SHOULD: - luceneQuery = query.toQuery(context); - break; - case FILTER: - case MUST_NOT: - luceneQuery = query.toFilter(context); - break; - } + Query luceneQuery = query.toQuery(context); booleanQueryBuilder.add(new BooleanClause(luceneQuery, occurs)); } } diff --git a/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java index e8d98a6b00b0e..9752265a70184 100644 --- a/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -129,7 +129,7 @@ public static ConstantScoreQueryBuilder fromXContent(XContentParser parser) thro @Override protected Query doToQuery(QueryShardContext context) throws IOException { - Query innerFilter = filterBuilder.toFilter(context); + Query innerFilter = filterBuilder.toQuery(context); return new ConstantScoreQuery(innerFilter); } diff --git a/server/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java index 93528bb952042..954107c656086 100644 --- a/server/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java @@ -326,9 +326,6 @@ public String getWriteableName() { protected Query doToQuery(QueryShardContext context) throws IOException { Query query = null; String rewrite = this.rewrite; - if (rewrite == null && context.isFilter()) { - rewrite = QueryParsers.CONSTANT_SCORE.getPreferredName(); - } MappedFieldType fieldType = context.fieldMapper(fieldName); if (fieldType != null) { query = fieldType.fuzzyQuery(value, fuzziness, prefixLength, maxExpansions, transpositions); diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/QueryBuilder.java index 5b765c5cbda5d..19adfebafabc5 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryBuilder.java @@ -37,16 +37,6 @@ public interface QueryBuilder extends NamedWriteable, ToXContentObject, Rewritea */ Query toQuery(QueryShardContext context) throws IOException; - /** - * Converts this QueryBuilder to an unscored lucene {@link Query} that acts as a filter. - * Returns {@code null} if this query should be ignored in the context of - * parent queries. - * - * @param context additional information needed to construct the queries - * @return the {@link Query} or {@code null} if this query should be ignored upstream - */ - Query toFilter(QueryShardContext context) throws IOException; - /** * Sets the arbitrary name to be assigned to the query (see named queries). * Implementers should return the concrete type of the diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index ac19298ae322d..82bae93e84d49 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -97,7 +97,6 @@ public String[] getTypes() { private boolean allowUnmappedFields; private boolean mapUnmappedFieldAsString; private NestedScope nestedScope; - private boolean isFilter; public QueryShardContext(int shardId, IndexSettings indexSettings, BitsetFilterCache bitsetFilterCache, BiFunction> indexFieldDataLookup, MapperService mapperService, @@ -132,7 +131,6 @@ private void reset() { this.lookup = null; this.namedQueries.clear(); this.nestedScope = new NestedScope(); - this.isFilter = false; } public IndexAnalyzers getIndexAnalyzers() { @@ -178,22 +176,6 @@ public Map copyNamedQueries() { return unmodifiableMap(new HashMap<>(namedQueries)); } - /** - * Return whether we are currently parsing a filter or a query. - */ - public boolean isFilter() { - return isFilter; - } - - /** - * Public for testing only! - * - * Sets whether we are currently parsing a filter or a query - */ - public void setIsFilter(boolean isFilter) { - this.isFilter = isFilter; - } - /** * Returns all the fields that match a given pattern. If prefixed with a * type then the fields will be returned with a type prefix. @@ -289,16 +271,6 @@ public Version indexVersionCreated() { return indexSettings.getIndexVersionCreated(); } - public ParsedQuery toFilter(QueryBuilder queryBuilder) { - return toQuery(queryBuilder, q -> { - Query filter = q.toFilter(this); - if (filter == null) { - return null; - } - return filter; - }); - } - public ParsedQuery toQuery(QueryBuilder queryBuilder) { return toQuery(queryBuilder, q -> { Query query = q.toQuery(this); diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java index d43c8120fe0c5..087718ed1bccb 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java @@ -346,11 +346,6 @@ public Query toQuery(QueryShardContext context) throws IOException { throw new UnsupportedOperationException(); } - @Override - public Query toFilter(QueryShardContext context) throws IOException { - throw new UnsupportedOperationException(); - } - @Override public String queryName() { throw new UnsupportedOperationException(); diff --git a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index 0b17ec72fbb17..091fd5f8c85e0 100644 --- a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -244,7 +244,7 @@ public void preProcess(boolean rewrite) { // initialize the filtering alias based on the provided filters try { final QueryBuilder queryBuilder = request.getAliasFilter().getQueryBuilder(); - aliasFilter = queryBuilder == null ? null : queryBuilder.toFilter(queryShardContext); + aliasFilter = queryBuilder == null ? null : queryBuilder.toQuery(queryShardContext); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregatorFactory.java index 69bc2de39dca9..7be588bcb5607 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregatorFactory.java @@ -51,7 +51,7 @@ public AdjacencyMatrixAggregatorFactory(String name, List filters, for (int i = 0; i < filters.size(); ++i) { KeyedFilter keyedFilter = filters.get(i); this.keys[i] = keyedFilter.key(); - Query filter = keyedFilter.filter().toFilter(context.getQueryShardContext()); + Query filter = keyedFilter.filter().toQuery(context.getQueryShardContext()); this.weights[i] = contextSearcher.createWeight(contextSearcher.rewrite(filter), ScoreMode.COMPLETE_NO_SCORES, 1f); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java index c7d500e81ca26..04dd8d3a53cea 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java @@ -43,7 +43,7 @@ public class FilterAggregatorFactory extends AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, context, parent, subFactoriesBuilder, metaData); - filter = filterBuilder.toFilter(context.getQueryShardContext()); + filter = filterBuilder.toQuery(context.getQueryShardContext()); } /** diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java index 81a78632d4bd6..cc299765621aa 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java @@ -56,7 +56,7 @@ public FiltersAggregatorFactory(String name, List filters, boolean for (int i = 0; i < filters.size(); ++i) { KeyedFilter keyedFilter = filters.get(i); this.keys[i] = keyedFilter.key(); - this.filters[i] = keyedFilter.filter().toFilter(context.getQueryShardContext()); + this.filters[i] = keyedFilter.filter().toQuery(context.getQueryShardContext()); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java index 3a424b0055f7a..4054f3796d9b5 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java @@ -96,7 +96,7 @@ public SignificantTermsAggregatorFactory(String name, this.executionHint = executionHint; this.filter = filterBuilder == null ? null - : filterBuilder.toFilter(context.getQueryShardContext()); + : filterBuilder.toQuery(context.getQueryShardContext()); IndexSearcher searcher = context.searcher(); this.supersetNumDocs = filter == null // Important - need to use the doc count that includes deleted docs diff --git a/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java index a7861dee9bba0..abef6d8ebec33 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java @@ -226,9 +226,9 @@ private static Query resolveNestedQuery(QueryShardContext context, NestedSortBui assert nestedFilter == Rewriteable.rewrite(nestedFilter, context) : "nested filter is not rewritten"; if (parentQuery == null) { // this is for back-compat, original single level nested sorting never applied a nested type filter - childQuery = nestedFilter.toFilter(context); + childQuery = nestedFilter.toQuery(context); } else { - childQuery = Queries.filtered(nestedObjectMapper.nestedTypeFilter(), nestedFilter.toFilter(context)); + childQuery = Queries.filtered(nestedObjectMapper.nestedTypeFilter(), nestedFilter.toQuery(context)); } } else { childQuery = nestedObjectMapper.nestedTypeFilter(); diff --git a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 46c9f56c33a95..6288c3c95666f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -21,7 +21,6 @@ import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParsingException; @@ -41,7 +40,6 @@ import java.util.Map; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; -import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; @@ -176,26 +174,6 @@ public void testEmptyBooleanQuery() throws Exception { } } - public void testDefaultMinShouldMatch() throws Exception { - // Queries have a minShouldMatch of 0 - BooleanQuery bq = (BooleanQuery) parseQuery(boolQuery().must(termQuery("foo", "bar"))).toQuery(createShardContext()); - assertEquals(0, bq.getMinimumNumberShouldMatch()); - - bq = (BooleanQuery) parseQuery(boolQuery().should(termQuery("foo", "bar"))).toQuery(createShardContext()); - assertEquals(0, bq.getMinimumNumberShouldMatch()); - - // Filters have a minShouldMatch of 0/1 - ConstantScoreQuery csq = (ConstantScoreQuery) parseQuery(constantScoreQuery(boolQuery() - .must(termQuery("foo", "bar")))).toQuery(createShardContext()); - bq = (BooleanQuery) csq.getQuery(); - assertEquals(0, bq.getMinimumNumberShouldMatch()); - - csq = (ConstantScoreQuery) parseQuery(constantScoreQuery(boolQuery().should(termQuery("foo", "bar")))) - .toQuery(createShardContext()); - bq = (BooleanQuery) csq.getQuery(); - assertEquals(1, bq.getMinimumNumberShouldMatch()); - } - public void testMinShouldMatchFilterWithoutShouldClauses() throws Exception { BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); boolQueryBuilder.filter(new BoolQueryBuilder().must(new MatchAllQueryBuilder())); @@ -216,29 +194,6 @@ public void testMinShouldMatchFilterWithoutShouldClauses() throws Exception { assertThat(innerBooleanClause.getQuery(), instanceOf(MatchAllDocsQuery.class)); } - public void testMinShouldMatchFilterWithShouldClauses() throws Exception { - BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); - boolQueryBuilder.filter(new BoolQueryBuilder().must(new MatchAllQueryBuilder()).should(new MatchAllQueryBuilder())); - Query query = boolQueryBuilder.toQuery(createShardContext()); - assertThat(query, instanceOf(BooleanQuery.class)); - BooleanQuery booleanQuery = (BooleanQuery) query; - assertThat(booleanQuery.getMinimumNumberShouldMatch(), equalTo(0)); - assertThat(booleanQuery.clauses().size(), equalTo(1)); - BooleanClause booleanClause = booleanQuery.clauses().get(0); - assertThat(booleanClause.getOccur(), equalTo(BooleanClause.Occur.FILTER)); - assertThat(booleanClause.getQuery(), instanceOf(BooleanQuery.class)); - BooleanQuery innerBooleanQuery = (BooleanQuery) booleanClause.getQuery(); - //we didn't set minimum should match initially, but there are should clauses so it should be 1 - assertThat(innerBooleanQuery.getMinimumNumberShouldMatch(), equalTo(1)); - assertThat(innerBooleanQuery.clauses().size(), equalTo(2)); - BooleanClause innerBooleanClause1 = innerBooleanQuery.clauses().get(0); - assertThat(innerBooleanClause1.getOccur(), equalTo(BooleanClause.Occur.MUST)); - assertThat(innerBooleanClause1.getQuery(), instanceOf(MatchAllDocsQuery.class)); - BooleanClause innerBooleanClause2 = innerBooleanQuery.clauses().get(1); - assertThat(innerBooleanClause2.getOccur(), equalTo(BooleanClause.Occur.SHOULD)); - assertThat(innerBooleanClause2.getQuery(), instanceOf(MatchAllDocsQuery.class)); - } - public void testMinShouldMatchBiggerThanNumberOfShouldClauses() throws Exception { BooleanQuery bq = (BooleanQuery) parseQuery( boolQuery() diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java index a4bbd1989dabc..47db7d42d8cd0 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java @@ -114,11 +114,6 @@ public Query toQuery(QueryShardContext context) throws IOException { return new TermQuery(new Term("foo", "bar")); } - @Override - public Query toFilter(QueryShardContext context) throws IOException { - return toQuery(context); - } - @Override public QueryBuilder queryName(String queryName) { return this; diff --git a/server/src/test/java/org/elasticsearch/index/query/plugin/CustomQueryParserIT.java b/server/src/test/java/org/elasticsearch/index/query/plugin/CustomQueryParserIT.java index 7df1f768bc991..f1ebcd971741e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/plugin/CustomQueryParserIT.java +++ b/server/src/test/java/org/elasticsearch/index/query/plugin/CustomQueryParserIT.java @@ -19,10 +19,6 @@ package org.elasticsearch.index.query.plugin; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.Query; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.indices.IndicesService; @@ -33,10 +29,7 @@ import java.util.Arrays; import java.util.Collection; -import static org.elasticsearch.index.query.QueryBuilders.boolQuery; -import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; -import static org.hamcrest.Matchers.instanceOf; public class CustomQueryParserIT extends ESIntegTestCase { @Override @@ -77,65 +70,4 @@ private static QueryShardContext queryShardContext() { return indicesService.indexServiceSafe(resolveIndex("index")).newQueryShardContext( randomInt(20), null, () -> { throw new UnsupportedOperationException(); }, null); } - - //see #11120 - public void testConstantScoreParsesFilter() throws Exception { - Query q = constantScoreQuery(new DummyQueryBuilder()).toQuery(queryShardContext()); - Query inner = ((ConstantScoreQuery) q).getQuery(); - assertThat(inner, instanceOf(DummyQueryParserPlugin.DummyQuery.class)); - assertEquals(true, ((DummyQueryParserPlugin.DummyQuery) inner).isFilter); - } - - //see #11120 - public void testBooleanParsesFilter() throws Exception { - // single clause, serialized as inner object - Query q = boolQuery() - .should(new DummyQueryBuilder()) - .must(new DummyQueryBuilder()) - .filter(new DummyQueryBuilder()) - .mustNot(new DummyQueryBuilder()).toQuery(queryShardContext()); - assertThat(q, instanceOf(BooleanQuery.class)); - BooleanQuery bq = (BooleanQuery) q; - assertEquals(4, bq.clauses().size()); - for (BooleanClause clause : bq.clauses()) { - DummyQueryParserPlugin.DummyQuery dummy = (DummyQueryParserPlugin.DummyQuery) clause.getQuery(); - switch (clause.getOccur()) { - case FILTER: - case MUST_NOT: - assertEquals(true, dummy.isFilter); - break; - case MUST: - case SHOULD: - assertEquals(false, dummy.isFilter); - break; - default: - throw new AssertionError(); - } - } - - // multiple clauses, serialized as inner arrays - q = boolQuery() - .should(new DummyQueryBuilder()).should(new DummyQueryBuilder()) - .must(new DummyQueryBuilder()).must(new DummyQueryBuilder()) - .filter(new DummyQueryBuilder()).filter(new DummyQueryBuilder()) - .mustNot(new DummyQueryBuilder()).mustNot(new DummyQueryBuilder()).toQuery(queryShardContext()); - assertThat(q, instanceOf(BooleanQuery.class)); - bq = (BooleanQuery) q; - assertEquals(8, bq.clauses().size()); - for (BooleanClause clause : bq.clauses()) { - DummyQueryParserPlugin.DummyQuery dummy = (DummyQueryParserPlugin.DummyQuery) clause.getQuery(); - switch (clause.getOccur()) { - case FILTER: - case MUST_NOT: - assertEquals(true, dummy.isFilter); - break; - case MUST: - case SHOULD: - assertEquals(false, dummy.isFilter); - break; - default: - throw new AssertionError(); - } - } - } } diff --git a/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java b/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java index cbd76877ce49a..16f7726a87732 100644 --- a/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java +++ b/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java @@ -58,7 +58,7 @@ public static DummyQueryBuilder fromXContent(XContentParser parser) throws IOExc @Override protected Query doToQuery(QueryShardContext context) throws IOException { - return new DummyQuery(context.isFilter()); + return new DummyQuery(); } @Override @@ -75,4 +75,4 @@ protected boolean doEquals(DummyQueryBuilder other) { public String getWriteableName() { return NAME; } -} \ No newline at end of file +} diff --git a/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java b/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java index 02653dcfd0e4d..3f57712a51b7e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java +++ b/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java @@ -40,13 +40,8 @@ public List> getQueries() { } public static class DummyQuery extends Query { - public final boolean isFilter; private final Query matchAllDocsQuery = new MatchAllDocsQuery(); - public DummyQuery(boolean isFilter) { - this.isFilter = isFilter; - } - @Override public String toString(String field) { return getClass().getSimpleName(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java index f3d057d8e8cd0..d92fb7ff62e43 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java @@ -23,24 +23,17 @@ import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexReader; -import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.RandomIndexWriter; -import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; -import org.apache.lucene.search.Query; import org.apache.lucene.store.Directory; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; -import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.AggregatorTestCase; -import org.hamcrest.Matchers; import org.junit.Before; -import java.io.IOException; - public class FilterAggregatorTests extends AggregatorTestCase { private MappedFieldType fieldType; @@ -107,22 +100,4 @@ public void testRandom() throws Exception { indexReader.close(); directory.close(); } - - public void testParsedAsFilter() throws IOException { - IndexReader indexReader = new MultiReader(); - IndexSearcher indexSearcher = newSearcher(indexReader); - QueryBuilder filter = QueryBuilders.boolQuery() - .must(QueryBuilders.termQuery("field", "foo")) - .should(QueryBuilders.termQuery("field", "bar")); - FilterAggregationBuilder builder = new FilterAggregationBuilder("test", filter); - AggregatorFactory factory = createAggregatorFactory(builder, indexSearcher, fieldType); - assertThat(factory, Matchers.instanceOf(FilterAggregatorFactory.class)); - FilterAggregatorFactory filterFactory = (FilterAggregatorFactory) factory; - Query parsedQuery = filterFactory.getWeight().getQuery(); - assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class)); - assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size()); - // means the bool query has been parsed as a filter, if it was a query minShouldMatch would - // be 0 - assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch()); - } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java index 6fdf207249f43..05ed091978270 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java @@ -23,23 +23,17 @@ import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexReader; -import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.RandomIndexWriter; -import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; -import org.apache.lucene.search.Query; import org.apache.lucene.store.Directory; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; -import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.AggregatorTestCase; -import org.hamcrest.Matchers; import org.junit.Before; -import java.io.IOException; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -203,22 +197,4 @@ public void testRandom() throws Exception { indexReader.close(); directory.close(); } - - public void testParsedAsFilter() throws IOException { - IndexReader indexReader = new MultiReader(); - IndexSearcher indexSearcher = newSearcher(indexReader); - QueryBuilder filter = QueryBuilders.boolQuery() - .must(QueryBuilders.termQuery("field", "foo")) - .should(QueryBuilders.termQuery("field", "bar")); - FiltersAggregationBuilder builder = new FiltersAggregationBuilder("test", filter); - AggregatorFactory factory = createAggregatorFactory(builder, indexSearcher, fieldType); - assertThat(factory, Matchers.instanceOf(FiltersAggregatorFactory.class)); - FiltersAggregatorFactory filtersFactory = (FiltersAggregatorFactory) factory; - Query parsedQuery = filtersFactory.getWeights()[0].getQuery(); - assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class)); - assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size()); - // means the bool query has been parsed as a filter, if it was a query minShouldMatch would - // be 0 - assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch()); - } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java index 0485d4f58550f..72403f8f7820b 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java @@ -28,11 +28,8 @@ import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; -import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.Term; -import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.IndexSearcher; -import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; @@ -46,12 +43,9 @@ import org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; -import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.bucket.significant.SignificantTermsAggregatorFactory.ExecutionMode; import org.elasticsearch.search.aggregations.bucket.terms.IncludeExclude; -import org.elasticsearch.search.aggregations.support.ValueType; -import org.hamcrest.Matchers; import org.junit.Before; import java.io.IOException; @@ -86,28 +80,6 @@ protected Map getFieldAliases(MappedFieldType... fieldT Function.identity())); } - public void testParsedAsFilter() throws IOException { - IndexReader indexReader = new MultiReader(); - IndexSearcher indexSearcher = newSearcher(indexReader); - QueryBuilder filter = QueryBuilders.boolQuery() - .must(QueryBuilders.termQuery("field", "foo")) - .should(QueryBuilders.termQuery("field", "bar")); - SignificantTermsAggregationBuilder builder = new SignificantTermsAggregationBuilder( - "test", ValueType.STRING) - .field("field") - .backgroundFilter(filter); - AggregatorFactory factory = createAggregatorFactory(builder, indexSearcher, fieldType); - assertThat(factory, Matchers.instanceOf(SignificantTermsAggregatorFactory.class)); - SignificantTermsAggregatorFactory sigTermsFactory = - (SignificantTermsAggregatorFactory) factory; - Query parsedQuery = sigTermsFactory.filter; - assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class)); - assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size()); - // means the bool query has been parsed as a filter, if it was a query minShouldMatch would - // be 0 - assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch()); - } - /** * Uses the significant terms aggregation to find the keywords in text fields */ diff --git a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java index cdf4a68b52f18..3d230bf67f019 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java @@ -76,7 +76,6 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.InternalAggregationTestCase; import org.junit.After; -import org.mockito.Matchers; import java.io.IOException; import java.util.ArrayList; @@ -306,8 +305,6 @@ protected QueryShardContext queryShardContextMock(MapperService mapperService) { QueryShardContext queryShardContext = mock(QueryShardContext.class); when(queryShardContext.getMapperService()).thenReturn(mapperService); NestedScope nestedScope = new NestedScope(); - when(queryShardContext.isFilter()).thenCallRealMethod(); - Mockito.doCallRealMethod().when(queryShardContext).setIsFilter(Matchers.anyBoolean()); when(queryShardContext.nestedScope()).thenReturn(nestedScope); return queryShardContext; } diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 2f46b8a887c87..40c23b9d5b3fb 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -466,12 +466,6 @@ public void testToQuery() throws IOException { assertNotEquals("modifying the boost doesn't affect the corresponding lucene query", rewrite(firstLuceneQuery), rewrite(thirdLuceneQuery)); } - - // check that context#isFilter is not changed by invoking toQuery/rewrite - boolean filterFlag = randomBoolean(); - context.setIsFilter(filterFlag); - rewriteQuery(firstQuery, context).toQuery(context); - assertEquals("isFilter should be unchanged", filterFlag, context.isFilter()); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java index dbb359bb70f1a..a8651701448d2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java @@ -136,7 +136,7 @@ protected DirectoryReader wrap(DirectoryReader reader) { QueryBuilder queryBuilder = queryShardContext.parseInnerQueryBuilder(parser); verifyRoleQuery(queryBuilder); failIfQueryUsesClient(queryBuilder, queryShardContext); - Query roleQuery = queryShardContext.toFilter(queryBuilder).query(); + Query roleQuery = queryShardContext.toQuery(queryBuilder).query(); filter.add(roleQuery, SHOULD); if (queryShardContext.getMapperService().hasNested()) { NestedHelper nestedHelper = new NestedHelper(queryShardContext.getMapperService()); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperIntegrationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperIntegrationTests.java index 90b0c6ee77362..ff132894af8ed 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperIntegrationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperIntegrationTests.java @@ -142,7 +142,7 @@ protected IndicesAccessControl getIndicesAccessControl() { for (int i = 0; i < numValues; i++) { ParsedQuery parsedQuery = new ParsedQuery(new TermQuery(new Term("field", values[i]))); doReturn(new TermQueryBuilder("field", values[i])).when(queryShardContext).parseInnerQueryBuilder(any(XContentParser.class)); - when(queryShardContext.toFilter(new TermsQueryBuilder("field", values[i]))).thenReturn(parsedQuery); + when(queryShardContext.toQuery(new TermsQueryBuilder("field", values[i]))).thenReturn(parsedQuery); DirectoryReader wrappedDirectoryReader = wrapper.wrap(directoryReader); IndexSearcher indexSearcher = wrapper.wrap(new IndexSearcher(wrappedDirectoryReader)); From 36ddca7d0ccf1af058213ffef1d29a0a2c696673 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 3 Dec 2018 13:45:19 +0100 Subject: [PATCH 04/17] Disable merges in testReuseInFileBasedPeerRecovery The test assumes lucene files don't change. Closes #35772 --- .../elasticsearch/gateway/RecoveryFromGatewayIT.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java b/server/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java index a8f2cfab2b79b..9d86df73e70a4 100644 --- a/server/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java +++ b/server/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java @@ -36,6 +36,7 @@ import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.MergePolicyConfig; import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.shard.ShardId; @@ -46,6 +47,7 @@ import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.test.ESIntegTestCase.Scope; +import org.elasticsearch.test.InternalSettingsPlugin; import org.elasticsearch.test.InternalTestCluster; import org.elasticsearch.test.InternalTestCluster.RestartCallback; import org.elasticsearch.test.store.MockFSIndexStore; @@ -81,7 +83,7 @@ public class RecoveryFromGatewayIT extends ESIntegTestCase { @Override protected Collection> nodePlugins() { - return Arrays.asList(MockFSIndexStore.TestPlugin.class); + return Arrays.asList(MockFSIndexStore.TestPlugin.class, InternalSettingsPlugin.class); } public void testOneNodeRecoverFromGateway() throws Exception { @@ -404,8 +406,12 @@ public void testReuseInFileBasedPeerRecovery() throws Exception { .admin() .indices() .prepareCreate("test") - .setSettings(Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 1)) - .get(); + .setSettings(Settings.builder() + .put("number_of_shards", 1) + .put("number_of_replicas", 1) + // disable merges to keep segments the same + .put(MergePolicyConfig.INDEX_MERGE_ENABLED, "false") + ).get(); logger.info("--> indexing docs"); int numDocs = randomIntBetween(1, 1024); From b5cae0af58caaae726c6d980665ac95282e77bfb Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Mon, 3 Dec 2018 13:55:18 +0100 Subject: [PATCH 05/17] Enforce max_buckets limit only in the final reduction phase (#36152) Given that we check the max buckets limit on each shard when collecting the buckets, and that non final reduction cannot add buckets (see #35921), there is no point in counting and checking the number of buckets as part of non final reduction phases. Such check is still needed though in the final reduction phases to make sure that the number of returned buckets is not above the allowed threshold. Relates somehow to #32125 as we will make use of non final reduction phases in CCS alternate execution mode and that increases the chance that this check trips for nothing when reducing aggs in each remote cluster. --- .../elasticsearch/search/SearchService.java | 3 +- .../search/SearchServiceTests.java | 32 +++++++++++++------ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 29afce4e08610..663214f49d8f4 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -1090,7 +1090,8 @@ public IndicesService getIndicesService() { } public InternalAggregation.ReduceContext createReduceContext(boolean finalReduce) { - return new InternalAggregation.ReduceContext(bigArrays, scriptService, multiBucketConsumerService.create(), finalReduce); + return new InternalAggregation.ReduceContext(bigArrays, scriptService, + finalReduce ? multiBucketConsumerService.create() : bucketCount -> {}, finalReduce); } public static final class CanMatchResponse extends SearchPhaseResult { diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index 50f654f4f497f..45adc1149a3eb 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.search; import com.carrotsearch.hppc.IntArrayList; - import org.apache.lucene.search.Query; import org.apache.lucene.store.AlreadyClosedException; import org.elasticsearch.action.ActionListener; @@ -59,6 +58,8 @@ import org.elasticsearch.script.MockScriptPlugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; +import org.elasticsearch.search.aggregations.InternalAggregation; +import org.elasticsearch.search.aggregations.MultiBucketConsumerService; import org.elasticsearch.search.aggregations.bucket.global.GlobalAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; @@ -155,7 +156,7 @@ protected Settings nodeSettings() { return Settings.builder().put("search.default_search_timeout", "5s").build(); } - public void testClearOnClose() throws ExecutionException, InterruptedException { + public void testClearOnClose() { createIndex("index"); client().prepareIndex("index", "type", "1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); SearchResponse searchResponse = client().prepareSearch("index").setSize(1).setScroll("1m").get(); @@ -167,7 +168,7 @@ public void testClearOnClose() throws ExecutionException, InterruptedException { assertEquals(0, service.getActiveContexts()); } - public void testClearOnStop() throws ExecutionException, InterruptedException { + public void testClearOnStop() { createIndex("index"); client().prepareIndex("index", "type", "1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); SearchResponse searchResponse = client().prepareSearch("index").setSize(1).setScroll("1m").get(); @@ -179,7 +180,7 @@ public void testClearOnStop() throws ExecutionException, InterruptedException { assertEquals(0, service.getActiveContexts()); } - public void testClearIndexDelete() throws ExecutionException, InterruptedException { + public void testClearIndexDelete() { createIndex("index"); client().prepareIndex("index", "type", "1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); SearchResponse searchResponse = client().prepareSearch("index").setSize(1).setScroll("1m").get(); @@ -208,7 +209,7 @@ public void testCloseSearchContextOnRewriteException() { assertEquals(activeRefs, indexShard.store().refCount()); } - public void testSearchWhileIndexDeleted() throws IOException, InterruptedException { + public void testSearchWhileIndexDeleted() throws InterruptedException { createIndex("index"); client().prepareIndex("index", "type", "1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); @@ -443,15 +444,15 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) { } @Override - protected void doWriteTo(StreamOutput out) throws IOException { + protected void doWriteTo(StreamOutput out) { } @Override - protected void doXContent(XContentBuilder builder, Params params) throws IOException { + protected void doXContent(XContentBuilder builder, Params params) { } @Override - protected Query doToQuery(QueryShardContext context) throws IOException { + protected Query doToQuery(QueryShardContext context) { return null; } @@ -501,7 +502,6 @@ public void testCanMatch() throws IOException { assertFalse(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchNoneQueryBuilder()), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults, null, null))); - } public void testCanRewriteToMatchNone() { @@ -519,7 +519,6 @@ public void testCanRewriteToMatchNone() { .suggest(new SuggestBuilder()))); assertFalse(SearchService.canRewriteToMatchNone(new SearchSourceBuilder().query(new TermQueryBuilder("foo", "bar")) .suggest(new SuggestBuilder()))); - } public void testSetSearchThrottled() { @@ -568,4 +567,17 @@ public void testExpandSearchThrottled() { assertHitCount(client().prepareSearch().get(), 0L); assertHitCount(client().prepareSearch().setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_FORBID_CLOSED).get(), 1L); } + + public void testCreateReduceContext() { + final SearchService service = getInstanceFromNode(SearchService.class); + { + InternalAggregation.ReduceContext reduceContext = service.createReduceContext(true); + expectThrows(MultiBucketConsumerService.TooManyBucketsException.class, + () -> reduceContext.consumeBucketsAndMaybeBreak(MultiBucketConsumerService.DEFAULT_MAX_BUCKETS + 1)); + } + { + InternalAggregation.ReduceContext reduceContext = service.createReduceContext(false); + reduceContext.consumeBucketsAndMaybeBreak(MultiBucketConsumerService.DEFAULT_MAX_BUCKETS + 1); + } + } } From 9c1c46a02f8ba4b3850578e7f4764ad58d0515e4 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 3 Dec 2018 08:55:48 -0500 Subject: [PATCH 06/17] TEST: Adjust min_retained_seq_no expectation min_retained_seq_no is non-negative, however, if the number of retained operations is greater than 0, then the expectation may be negative. --- .../engine/CombinedDeletionPolicyTests.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java b/server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java index 3f9fc9a0429b7..6546eaa124483 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java @@ -89,8 +89,8 @@ public void testKeepCommitsAfterGlobalCheckpoint() throws Exception { } assertThat(translogPolicy.getMinTranslogGenerationForRecovery(), equalTo(translogGenList.get(keptIndex))); assertThat(translogPolicy.getTranslogGenerationOfLastCommit(), equalTo(lastTranslogGen)); - assertThat(softDeletesPolicy.getMinRetainedSeqNo(), - equalTo(Math.min(getLocalCheckpoint(commitList.get(keptIndex)) + 1, globalCheckpoint.get() + 1 - extraRetainedOps))); + assertThat(softDeletesPolicy.getMinRetainedSeqNo(), equalTo( + Math.max(0, Math.min(getLocalCheckpoint(commitList.get(keptIndex)) + 1, globalCheckpoint.get() + 1 - extraRetainedOps)))); } public void testAcquireIndexCommit() throws Exception { @@ -125,8 +125,8 @@ public void testAcquireIndexCommit() throws Exception { commitList.forEach(this::resetDeletion); indexPolicy.onCommit(commitList); IndexCommit safeCommit = CombinedDeletionPolicy.findSafeCommitPoint(commitList, globalCheckpoint.get()); - assertThat(softDeletesPolicy.getMinRetainedSeqNo(), - equalTo(Math.min(getLocalCheckpoint(safeCommit) + 1, globalCheckpoint.get() + 1 - extraRetainedOps))); + assertThat(softDeletesPolicy.getMinRetainedSeqNo(), equalTo( + Math.max(0, Math.min(getLocalCheckpoint(safeCommit) + 1, globalCheckpoint.get() + 1 - extraRetainedOps)))); // Captures and releases some commits int captures = between(0, 5); for (int n = 0; n < captures; n++) { @@ -156,8 +156,8 @@ public void testAcquireIndexCommit() throws Exception { equalTo(Long.parseLong(commitList.get(safeIndex).getUserData().get(Translog.TRANSLOG_GENERATION_KEY)))); assertThat(translogPolicy.getTranslogGenerationOfLastCommit(), equalTo(Long.parseLong(commitList.get(commitList.size() - 1).getUserData().get(Translog.TRANSLOG_GENERATION_KEY)))); - assertThat(softDeletesPolicy.getMinRetainedSeqNo(), - equalTo(Math.min(getLocalCheckpoint(commitList.get(safeIndex)) + 1, globalCheckpoint.get() + 1 - extraRetainedOps))); + assertThat(softDeletesPolicy.getMinRetainedSeqNo(), equalTo( + Math.max(0, Math.min(getLocalCheckpoint(commitList.get(safeIndex)) + 1, globalCheckpoint.get() + 1 - extraRetainedOps)))); } snapshottingCommits.forEach(indexPolicy::releaseCommit); globalCheckpoint.set(randomLongBetween(lastMaxSeqNo, Long.MAX_VALUE)); @@ -170,8 +170,8 @@ public void testAcquireIndexCommit() throws Exception { assertThat(translogPolicy.getMinTranslogGenerationForRecovery(), equalTo(lastTranslogGen)); assertThat(translogPolicy.getTranslogGenerationOfLastCommit(), equalTo(lastTranslogGen)); IndexCommit safeCommit = CombinedDeletionPolicy.findSafeCommitPoint(commitList, globalCheckpoint.get()); - assertThat(softDeletesPolicy.getMinRetainedSeqNo(), - equalTo(Math.min(getLocalCheckpoint(safeCommit) + 1, globalCheckpoint.get() + 1 - extraRetainedOps))); + assertThat(softDeletesPolicy.getMinRetainedSeqNo(), equalTo( + Math.max(0, Math.min(getLocalCheckpoint(safeCommit) + 1, globalCheckpoint.get() + 1 - extraRetainedOps)))); } public void testLegacyIndex() throws Exception { From 433a506d06c73d227920261df06aa404de310e0b Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 3 Dec 2018 16:39:29 +0100 Subject: [PATCH 07/17] SNAPSHOT: Improve Resilience SnapshotShardService (#36113) * Resolve the index in the snapshotting thread * Added test for routing table - snapshot state mismatch --- .../snapshots/SnapshotShardsService.java | 4 +- .../DedicatedClusterSnapshotRestoreIT.java | 46 ++++++++++ .../BusyMasterServiceDisruption.java | 89 +++++++++++++++++++ 3 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 test/framework/src/main/java/org/elasticsearch/test/disruption/BusyMasterServiceDisruption.java diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java index c505a0a28b65d..7e590bc4104a9 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java @@ -323,15 +323,15 @@ private void processIndexShardSnapshots(ClusterChangedEvent event) { for (final Map.Entry shardEntry : entry.getValue().entrySet()) { final ShardId shardId = shardEntry.getKey(); - final IndexShard indexShard = indicesService.indexServiceSafe(shardId.getIndex()).getShardOrNull(shardId.id()); final IndexId indexId = indicesMap.get(shardId.getIndexName()); - assert indexId != null; executor.execute(new AbstractRunnable() { final SetOnce failure = new SetOnce<>(); @Override public void doRun() { + final IndexShard indexShard = indicesService.indexServiceSafe(shardId.getIndex()).getShardOrNull(shardId.id()); + assert indexId != null; snapshot(indexShard, snapshot, indexId, shardEntry.getValue()); } diff --git a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index 3347e2b8ee453..66a133c6abeb4 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -78,6 +78,8 @@ import org.elasticsearch.test.ESIntegTestCase.Scope; import org.elasticsearch.test.InternalTestCluster; import org.elasticsearch.test.TestCustomMetaData; +import org.elasticsearch.test.disruption.BusyMasterServiceDisruption; +import org.elasticsearch.test.disruption.ServiceDisruptionScheme; import org.elasticsearch.test.rest.FakeRestRequest; import java.io.IOException; @@ -1151,6 +1153,50 @@ public void testSnapshotTotalAndIncrementalSizes() throws IOException { assertThat(anotherStats.getTotalSize(), is(snapshot1FileSize)); } + public void testDataNodeRestartWithBusyMasterDuringSnapshot() throws Exception { + logger.info("--> starting a master node and two data nodes"); + internalCluster().startMasterOnlyNode(); + internalCluster().startDataOnlyNodes(2); + logger.info("--> creating repository"); + assertAcked(client().admin().cluster().preparePutRepository("test-repo") + .setType("mock").setSettings(Settings.builder() + .put("location", randomRepoPath()) + .put("compress", randomBoolean()) + .put("max_snapshot_bytes_per_sec", "1000b") + .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES))); + assertAcked(prepareCreate("test-idx", 0, Settings.builder() + .put("number_of_shards", 5).put("number_of_replicas", 0))); + ensureGreen(); + logger.info("--> indexing some data"); + final int numdocs = randomIntBetween(50, 100); + IndexRequestBuilder[] builders = new IndexRequestBuilder[numdocs]; + for (int i = 0; i < builders.length; i++) { + builders[i] = client().prepareIndex("test-idx", "type1", + Integer.toString(i)).setSource("field1", "bar " + i); + } + indexRandom(true, builders); + flushAndRefresh(); + final String dataNode = blockNodeWithIndex("test-repo", "test-idx"); + logger.info("--> snapshot"); + client(internalCluster().getMasterName()).admin().cluster() + .prepareCreateSnapshot("test-repo", "test-snap").setWaitForCompletion(false).setIndices("test-idx").get(); + ServiceDisruptionScheme disruption = new BusyMasterServiceDisruption(random(), Priority.HIGH); + setDisruptionScheme(disruption); + disruption.startDisrupting(); + logger.info("--> restarting data node, which should cause primary shards to be failed"); + internalCluster().restartNode(dataNode, InternalTestCluster.EMPTY_CALLBACK); + unblockNode("test-repo", dataNode); + disruption.stopDisrupting(); + // check that snapshot completes + assertBusy(() -> { + GetSnapshotsResponse snapshotsStatusResponse = client().admin().cluster() + .prepareGetSnapshots("test-repo").setSnapshots("test-snap").setIgnoreUnavailable(true).get(); + assertEquals(1, snapshotsStatusResponse.getSnapshots().size()); + SnapshotInfo snapshotInfo = snapshotsStatusResponse.getSnapshots().get(0); + assertTrue(snapshotInfo.state().toString(), snapshotInfo.state().completed()); + }, 30, TimeUnit.SECONDS); + } + private long calculateTotalFilesSize(List files) { return files.stream().mapToLong(f -> { try { diff --git a/test/framework/src/main/java/org/elasticsearch/test/disruption/BusyMasterServiceDisruption.java b/test/framework/src/main/java/org/elasticsearch/test/disruption/BusyMasterServiceDisruption.java new file mode 100644 index 0000000000000..3621cba1e7992 --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/test/disruption/BusyMasterServiceDisruption.java @@ -0,0 +1,89 @@ +/* + * 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.test.disruption; + +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Priority; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.test.InternalTestCluster; +import java.util.Random; +import java.util.concurrent.atomic.AtomicBoolean; + +public class BusyMasterServiceDisruption extends SingleNodeDisruption { + private final AtomicBoolean active = new AtomicBoolean(); + private final Priority priority; + + public BusyMasterServiceDisruption(Random random, Priority priority) { + super(random); + this.priority = priority; + } + + @Override + public void startDisrupting() { + disruptedNode = cluster.getMasterName(); + final String disruptionNodeCopy = disruptedNode; + if (disruptionNodeCopy == null) { + return; + } + ClusterService clusterService = cluster.getInstance(ClusterService.class, disruptionNodeCopy); + if (clusterService == null) { + return; + } + logger.info("making master service busy on node [{}] at priority [{}]", disruptionNodeCopy, priority); + active.set(true); + submitTask(clusterService); + } + + private void submitTask(ClusterService clusterService) { + clusterService.getMasterService().submitStateUpdateTask( + "service_disruption_block", + new ClusterStateUpdateTask(priority) { + @Override + public ClusterState execute(ClusterState currentState) { + if (active.get()) { + submitTask(clusterService); + } + return currentState; + } + + @Override + public void onFailure(String source, Exception e) { + logger.error("unexpected error during disruption", e); + } + } + ); + } + + @Override + public void stopDisrupting() { + active.set(false); + } + + @Override + public void removeAndEnsureHealthy(InternalTestCluster cluster) { + removeFromCluster(cluster); + } + + @Override + public TimeValue expectedTimeToHeal() { + return TimeValue.timeValueMinutes(0); + } +} From f8f521bad4b28f060e30d963f41c8ff6f9e6e8cd Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 3 Dec 2018 10:26:08 -0600 Subject: [PATCH 08/17] Deprecate /_xpack/monitoring/* in favor of /_monitoring/* (#36130) This commit is part of our plan to deprecate and ultimately remove the use of _xpack in the REST APIs. * Add deprecation for /_xpack/monitoring/_bulk in favor of /_monitoring/bulk * Removed xpack from the rest-api-spec and tests * Removed xpack from the Action name * Removed MonitoringRestHandler as an unnecessary abstraction * Minor corrections to comments Relates #35958 --- .../exporter/MonitoringTemplateUtils.java | 2 +- .../rest/MonitoringRestHandler.java | 19 --------------- .../rest/action/RestMonitoringBulkAction.java | 23 ++++++++++++------- .../AbstractIndicesCleanerTestCase.java | 3 +-- .../local/LocalExporterIntegTests.java | 2 +- .../monitoring/integration/MonitoringIT.java | 14 ++++------- .../action/RestMonitoringBulkActionTests.java | 2 +- ...itoring.bulk.json => monitoring.bulk.json} | 6 ++--- .../test/monitoring/bulk/10_basic.yml | 16 ++++++------- .../test/monitoring/bulk/20_privileges.yml | 4 ++-- 10 files changed, 36 insertions(+), 55 deletions(-) delete mode 100644 x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/MonitoringRestHandler.java rename x-pack/plugin/src/test/resources/rest-api-spec/api/{xpack.monitoring.bulk.json => monitoring.bulk.json} (86%) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/exporter/MonitoringTemplateUtils.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/exporter/MonitoringTemplateUtils.java index ad67ba723ca51..78e094cb8cefe 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/exporter/MonitoringTemplateUtils.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/exporter/MonitoringTemplateUtils.java @@ -35,7 +35,7 @@ public final class MonitoringTemplateUtils { */ public static final String TEMPLATE_VERSION = "6"; /** - * The previous version of templates, which we still support via the REST _xpack/monitoring/_bulk endpoint because + * The previous version of templates, which we still support via the REST /_monitoring/bulk endpoint because * nothing changed for those documents. */ public static final String OLD_TEMPLATE_VERSION = "2"; diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/MonitoringRestHandler.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/MonitoringRestHandler.java deleted file mode 100644 index a0e1f919f5a9b..0000000000000 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/MonitoringRestHandler.java +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.monitoring.rest; - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.xpack.core.rest.XPackRestHandler; - -public abstract class MonitoringRestHandler extends XPackRestHandler { - - protected static String URI_BASE = XPackRestHandler.URI_BASE + "/monitoring"; - - public MonitoringRestHandler(Settings settings) { - super(settings); - } - -} diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java index 06145a2339864..9df60f8c5ac73 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java @@ -5,8 +5,10 @@ */ package org.elasticsearch.xpack.monitoring.rest.action; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.rest.BytesRestResponse; @@ -19,7 +21,7 @@ import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkRequestBuilder; import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkResponse; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils; -import org.elasticsearch.xpack.monitoring.rest.MonitoringRestHandler; +import org.elasticsearch.xpack.core.rest.XPackRestHandler; import java.io.IOException; import java.util.Arrays; @@ -33,20 +35,25 @@ import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.rest.RestRequest.Method.PUT; -public class RestMonitoringBulkAction extends MonitoringRestHandler { +public class RestMonitoringBulkAction extends XPackRestHandler { public static final String MONITORING_ID = "system_id"; public static final String MONITORING_VERSION = "system_api_version"; public static final String INTERVAL = "interval"; - + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestMonitoringBulkAction.class)); private final Map> supportedApiVersions; public RestMonitoringBulkAction(Settings settings, RestController controller) { super(settings); - controller.registerHandler(POST, URI_BASE + "/_bulk", this); - controller.registerHandler(PUT, URI_BASE + "/_bulk", this); - controller.registerHandler(POST, URI_BASE + "/{type}/_bulk", this); - controller.registerHandler(PUT, URI_BASE + "/{type}/_bulk", this); + // TODO: remove deprecated endpoint in 8.0.0 + controller.registerWithDeprecatedHandler(POST, "/_monitoring/bulk", this, + POST, "/_xpack/monitoring/_bulk", deprecationLogger); + controller.registerWithDeprecatedHandler(PUT, "/_monitoring/bulk", this, + PUT, "/_xpack/monitoring/_bulk", deprecationLogger); + controller.registerWithDeprecatedHandler(POST, "/_monitoring/{type}/bulk", this, + POST, "/_xpack/monitoring/{type}/_bulk", deprecationLogger); + controller.registerWithDeprecatedHandler(PUT, "/_monitoring/{type}/bulk", this, + PUT, "/_xpack/monitoring/{type}/_bulk", deprecationLogger); final List allVersions = Arrays.asList( MonitoringTemplateUtils.TEMPLATE_VERSION, @@ -63,7 +70,7 @@ public RestMonitoringBulkAction(Settings settings, RestController controller) { @Override public String getName() { - return "xpack_monitoring_bulk_action"; + return "monitoring_bulk"; } @Override diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java index 23bb21a55ed24..762ab49bba73c 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java @@ -74,8 +74,7 @@ public void testDoesNotIgnoreIndicesInOtherVersions() throws Exception { createTimestampedIndex(now().minusYears(1), MonitoringTemplateUtils.OLD_TEMPLATE_VERSION); // In the past, this index would not be deleted, but starting in 6.x the monitoring cluster // will be required to be a newer template version than the production cluster, so the index - // pushed to it will never be "unknown" in terms of their version (relates to the - // _xpack/monitoring/_setup API) + // pushed to it will never be "unknown" in terms of their version createTimestampedIndex(now().minusDays(10), String.valueOf(Integer.MAX_VALUE)); // Won't be deleted diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java index 7bc035f7ae236..2c0ee6379e46f 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java @@ -84,7 +84,7 @@ public void testExport() throws Exception { indexRandom(true, indexRequestBuilders); } - // start the monitoring service so that _xpack/monitoring/_bulk is not ignored + // start the monitoring service so that /_monitoring/bulk is not ignored final Settings.Builder exporterSettings = Settings.builder() .put(MonitoringService.ENABLED.getKey(), true) .put("xpack.monitoring.exporters._local.enabled", true) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/integration/MonitoringIT.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/integration/MonitoringIT.java index 158f6a812626e..2b5b2882ab83a 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/integration/MonitoringIT.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/integration/MonitoringIT.java @@ -113,23 +113,17 @@ private String createBulkEntity() { } /** - * Monitoring Bulk API test: + * Monitoring Bulk test: * - * This test uses the Monitoring Bulk API to index document as an external application like Kibana would do. It - * then ensure that the documents were correctly indexed and have the expected information. + * This test uses the Monitoring Bulk Request to index documents. It then ensure that the documents were correctly + * indexed and have the expected information. REST API tests (like how this is really called) are handled as part of the + * XPackRest tests. */ public void testMonitoringBulk() throws Exception { whenExportersAreReady(() -> { final MonitoredSystem system = randomSystem(); final TimeValue interval = TimeValue.timeValueSeconds(randomIntBetween(1, 20)); - // REST is the realistic way that these operations happen, so it's the most realistic way to integration test it too - // Use Monitoring Bulk API to index 3 documents - //final Request bulkRequest = new Request("POST", "/_xpack/monitoring/_bulk"); - //< - //bulkRequest.setJsonEntity(createBulkEntity()); - //final Response bulkResponse = getRestClient().performRequest(request); - final MonitoringBulkResponse bulkResponse = new MonitoringBulkRequestBuilder(client()) .add(system, null, new BytesArray(createBulkEntity().getBytes("UTF-8")), XContentType.JSON, diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkActionTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkActionTests.java index 15a19c8a135cf..7a4427c9f0fdc 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkActionTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkActionTests.java @@ -52,7 +52,7 @@ public class RestMonitoringBulkActionTests extends ESTestCase { public void testGetName() { // Are you sure that you want to change the name? - assertThat(action.getName(), is("xpack_monitoring_bulk_action")); + assertThat(action.getName(), is("monitoring_bulk")); } public void testSupportsContentStream() { diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/api/xpack.monitoring.bulk.json b/x-pack/plugin/src/test/resources/rest-api-spec/api/monitoring.bulk.json similarity index 86% rename from x-pack/plugin/src/test/resources/rest-api-spec/api/xpack.monitoring.bulk.json rename to x-pack/plugin/src/test/resources/rest-api-spec/api/monitoring.bulk.json index 71f1b1fc13bf7..55ce7b9ba6170 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/api/xpack.monitoring.bulk.json +++ b/x-pack/plugin/src/test/resources/rest-api-spec/api/monitoring.bulk.json @@ -1,10 +1,10 @@ { - "xpack.monitoring.bulk": { + "monitoring.bulk": { "documentation": "http://www.elastic.co/guide/en/monitoring/current/appendix-api-bulk.html", "methods": ["POST", "PUT"], "url": { - "path": "/_xpack/monitoring/_bulk", - "paths": ["/_xpack/monitoring/_bulk", "/_xpack/monitoring/{type}/_bulk"], + "path": "/_monitoring/bulk", + "paths": ["/_monitoring/bulk", "/_monitoring/{type}/bulk"], "parts": { "type": { "type" : "string", diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/10_basic.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/10_basic.yml index c5d2285269249..37d2e5feda349 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/10_basic.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/10_basic.yml @@ -2,7 +2,7 @@ "Bulk indexing of monitoring data": - do: - xpack.monitoring.bulk: + monitoring.bulk: system_id: "kibana" system_api_version: "6" interval: "10s" @@ -37,7 +37,7 @@ - match: { hits.total: 2 } - do: - xpack.monitoring.bulk: + monitoring.bulk: system_id: "kibana" system_api_version: "6" interval: "123456ms" @@ -83,7 +83,7 @@ # Old system_api_version should still be accepted - do: - xpack.monitoring.bulk: + monitoring.bulk: system_id: "kibana" system_api_version: "2" interval: "10000ms" @@ -127,7 +127,7 @@ # Missing a system_id causes it to fail - do: catch: bad_request - xpack.monitoring.bulk: + monitoring.bulk: system_api_version: "6" interval: "10s" type: "default_type" @@ -138,7 +138,7 @@ # Missing a system_api_version causes it to fail - do: catch: bad_request - xpack.monitoring.bulk: + monitoring.bulk: system_id: "kibana" interval: "10s" type: "default_type" @@ -149,7 +149,7 @@ # Missing an interval causes it to fail - do: catch: bad_request - xpack.monitoring.bulk: + monitoring.bulk: system_id: "kibana" system_api_version: "6" type: "default_type" @@ -161,7 +161,7 @@ "Bulk indexing of monitoring data on closed indices should throw an export exception": - do: - xpack.monitoring.bulk: + monitoring.bulk: system_id: "beats" system_api_version: "6" interval: "5s" @@ -193,7 +193,7 @@ - do: catch: /export_exception/ - xpack.monitoring.bulk: + monitoring.bulk: system_id: "beats" system_api_version: "6" interval: "5s" diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/20_privileges.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/20_privileges.yml index 9f065bb55224f..07cd7d259365d 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/20_privileges.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/20_privileges.yml @@ -82,7 +82,7 @@ teardown: headers: # Authorization: logstash_agent Authorization: "Basic bG9nc3Rhc2hfYWdlbnQ6czNrcml0" - xpack.monitoring.bulk: + monitoring.bulk: system_id: "logstash" system_api_version: "6" interval: "10s" @@ -118,7 +118,7 @@ teardown: headers: # Authorization: unknown_agent Authorization: "Basic dW5rbm93bl9hZ2VudDpzM2tyaXQ=" - xpack.monitoring.bulk: + monitoring.bulk: system_id: "logstash" system_api_version: "6" interval: "10s" From da2dbcdf38e02ae72a202aeaa78cdc5d006dae73 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 3 Dec 2018 17:34:15 +0000 Subject: [PATCH 09/17] Improve painless docs for score, similarity, weight and sort (#35629) --- .../painless-score-context.asciidoc | 37 +++++++++++++++-- .../painless-similarity-context.asciidoc | 16 +++++++- .../painless-sort-context.asciidoc | 41 +++++++++++++++++-- .../painless-weight-context.asciidoc | 7 +++- 4 files changed, 92 insertions(+), 9 deletions(-) diff --git a/docs/painless/painless-contexts/painless-score-context.asciidoc b/docs/painless/painless-contexts/painless-score-context.asciidoc index bd1e1de7f777d..2bec9021c1720 100644 --- a/docs/painless/painless-contexts/painless-score-context.asciidoc +++ b/docs/painless/painless-contexts/painless-score-context.asciidoc @@ -11,8 +11,10 @@ score to documents returned from a query. User-defined parameters passed in as part of the query. `doc` (`Map`, read-only):: - Contains the fields of the current document where each field is a - `List` of values. + Contains the fields of the current document. For single-valued fields, + the value can be accessed via `doc['fieldname'].value`. For multi-valued + fields, this returns the first value; other values can be accessed + via `doc['fieldname'].get(index)` `_score` (`double` read-only):: The similarity score of the current document. @@ -24,4 +26,33 @@ score to documents returned from a query. *API* -The standard <> is available. \ No newline at end of file +The standard <> is available. + +*Example* + +To run this example, first follow the steps in +<>. + +The following query finds all unsold seats, with lower 'row' values +scored higher. + +[source,js] +-------------------------------------------------- +GET /seats/_search +{ + "query": { + "function_score": { + "query": { + "match": { "sold": "false" } + }, + "script_score" : { + "script" : { + "source": "1.0 / doc['row'].value" + } + } + } + } +} +-------------------------------------------------- +// CONSOLE +// TEST[setup:seats] \ No newline at end of file diff --git a/docs/painless/painless-contexts/painless-similarity-context.asciidoc b/docs/painless/painless-contexts/painless-similarity-context.asciidoc index 53b37be52b6d7..9a8e59350d1a8 100644 --- a/docs/painless/painless-contexts/painless-similarity-context.asciidoc +++ b/docs/painless/painless-contexts/painless-similarity-context.asciidoc @@ -15,6 +15,9 @@ documents in a query. `params` (`Map`, read-only):: User-defined parameters passed in at query-time. +`weight` (`float`, read-only):: + The weight as calculated by a {ref}/painless-weight-context[weight script] + `query.boost` (`float`, read-only):: The boost value if provided by the query. If this is not provided the value is `1.0f`. @@ -37,12 +40,23 @@ documents in a query. The total occurrences of the current term in the index. `doc.length` (`long`, read-only):: - The number of tokens the current document has in the current field. + The number of tokens the current document has in the current field. This + is decoded from the stored {ref}/norms[norms] and may be approximate for + long fields `doc.freq` (`long`, read-only):: The number of occurrences of the current term in the current document for the current field. +Note that the `query`, `field`, and `term` variables are also available to the +{ref}/painless-weight-context[weight context]. They are more efficiently used +there, as they are constant for all documents. + +For queries that contain multiple terms, the script is called once for each +term with that term's calculated weight, and the results are summed. Note that some +terms might have a `doc.freq` value of `0` on a document, for example if a query +uses synonyms. + *Return* `double`:: diff --git a/docs/painless/painless-contexts/painless-sort-context.asciidoc b/docs/painless/painless-contexts/painless-sort-context.asciidoc index 9efd507668839..64c17ad07a664 100644 --- a/docs/painless/painless-contexts/painless-sort-context.asciidoc +++ b/docs/painless/painless-contexts/painless-sort-context.asciidoc @@ -10,8 +10,10 @@ Use a Painless script to User-defined parameters passed in as part of the query. `doc` (`Map`, read-only):: - Contains the fields of the current document where each field is a - `List` of values. + Contains the fields of the current document. For single-valued fields, + the value can be accessed via `doc['fieldname'].value`. For multi-valued + fields, this returns the first value; other values can be accessed + via `doc['fieldname'].get(index)` `_score` (`double` read-only):: The similarity score of the current document. @@ -23,4 +25,37 @@ Use a Painless script to *API* -The standard <> is available. \ No newline at end of file +The standard <> is available. + +*Example* + +To run this example, first follow the steps in +<>. + +To sort results by the length of the `theatre` field, submit the following query: + +[source,js] +---- +GET /_search +{ + "query" : { + "term" : { "sold" : "true" } + }, + "sort" : { + "_script" : { + "type" : "number", + "script" : { + "lang": "painless", + "source": "doc['theatre'].value.length() * params.factor", + "params" : { + "factor" : 1.1 + } + }, + "order" : "asc" + } + } +} + +---- +// CONSOLE +// TEST[setup:seats] \ No newline at end of file diff --git a/docs/painless/painless-contexts/painless-weight-context.asciidoc b/docs/painless/painless-contexts/painless-weight-context.asciidoc index ad215d5386b05..319b7999aa831 100644 --- a/docs/painless/painless-contexts/painless-weight-context.asciidoc +++ b/docs/painless/painless-contexts/painless-weight-context.asciidoc @@ -3,8 +3,11 @@ Use a Painless script to create a {ref}/index-modules-similarity.html[weight] for use in a -<>. Weight is used to prevent -recalculation of constants that remain the same across documents. +<>. The weight makes up the +part of the similarity calculation that is independent of the document being +scored, and so can be built up front and cached. + +Queries that contain multiple terms calculate a separate weight for each term. *Variables* From 59ee8b5c691bec3e7cb784c7f2f390c0a7325896 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 3 Dec 2018 10:22:42 -0800 Subject: [PATCH 10/17] Remove the deprecated _termvector endpoint. (#36131) --- docs/reference/docs/termvectors.asciidoc | 3 - .../migration/migrate_7_0/api.asciidoc | 7 ++ .../migration/migrate_7_0/java.asciidoc | 8 ++- .../java/org/elasticsearch/client/Client.java | 33 --------- .../client/support/AbstractClient.java | 24 ------- .../document/RestTermVectorsAction.java | 16 ++--- .../action/termvectors/GetTermVectorsIT.java | 4 +- .../document/RestTermVectorsActionTests.java | 67 ------------------- .../integration/IndexPrivilegeTests.java | 4 +- 9 files changed, 22 insertions(+), 144 deletions(-) delete mode 100644 server/src/test/java/org/elasticsearch/rest/action/document/RestTermVectorsActionTests.java diff --git a/docs/reference/docs/termvectors.asciidoc b/docs/reference/docs/termvectors.asciidoc index 3cd21b21df4d6..11de3e5a27ff1 100644 --- a/docs/reference/docs/termvectors.asciidoc +++ b/docs/reference/docs/termvectors.asciidoc @@ -27,9 +27,6 @@ or by adding the requested fields in the request body (see example below). Fields can also be specified with wildcards in similar way to the <> -[WARNING] -Note that the usage of `/_termvector` is deprecated in 2.0, and replaced by `/_termvectors`. - [float] === Return values diff --git a/docs/reference/migration/migrate_7_0/api.asciidoc b/docs/reference/migration/migrate_7_0/api.asciidoc index a543ef4b0540c..83370a93d556a 100644 --- a/docs/reference/migration/migrate_7_0/api.asciidoc +++ b/docs/reference/migration/migrate_7_0/api.asciidoc @@ -119,3 +119,10 @@ while now an exception is thrown. The deprecated graph endpoints (those with `/_graph/_explore`) have been removed. + + +[float] +==== Deprecated `_termvector` endpoint removed + +The `_termvector` endpoint was deprecated in 2.0 and has now been removed. +The endpoint `_termvectors` (plural) should be used instead. diff --git a/docs/reference/migration/migrate_7_0/java.asciidoc b/docs/reference/migration/migrate_7_0/java.asciidoc index 4357b3fa72857..e48a4cf1b45c3 100644 --- a/docs/reference/migration/migrate_7_0/java.asciidoc +++ b/docs/reference/migration/migrate_7_0/java.asciidoc @@ -32,4 +32,10 @@ was moved to `org.elasticsearch.search.aggregations.PipelineAggregationBuilders` ==== `Retry.withBackoff` methods with `Settings` removed The variants of `Retry.withBackoff` that included `Settings` have been removed -because `Settings` is no longer needed. \ No newline at end of file +because `Settings` is no longer needed. + +[float] +==== Deprecated method `Client#termVector` removed + +The client method `termVector`, deprecated in 2.0, has been removed. The method +`termVectors` (plural) should be used instead. \ No newline at end of file diff --git a/server/src/main/java/org/elasticsearch/client/Client.java b/server/src/main/java/org/elasticsearch/client/Client.java index d2be1fba086df..07871709f5726 100644 --- a/server/src/main/java/org/elasticsearch/client/Client.java +++ b/server/src/main/java/org/elasticsearch/client/Client.java @@ -370,39 +370,6 @@ public interface Client extends ElasticsearchClient, Releasable { */ TermVectorsRequestBuilder prepareTermVectors(String index, String type, String id); - /** - * An action that returns the term vectors for a specific document. - * - * @param request The term vector request - * @return The response future - */ - @Deprecated - ActionFuture termVector(TermVectorsRequest request); - - /** - * An action that returns the term vectors for a specific document. - * - * @param request The term vector request - */ - @Deprecated - void termVector(TermVectorsRequest request, ActionListener listener); - - /** - * Builder for the term vector request. - */ - @Deprecated - TermVectorsRequestBuilder prepareTermVector(); - - /** - * Builder for the term vector request. - * - * @param index The index to load the document from - * @param type The type of the document - * @param id The id of the document - */ - @Deprecated - TermVectorsRequestBuilder prepareTermVector(String index, String type, String id); - /** * Multi get term vectors. */ diff --git a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java index d6ce608901714..d642101e1c3e9 100644 --- a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java @@ -581,30 +581,6 @@ public TermVectorsRequestBuilder prepareTermVectors(String index, String type, S return new TermVectorsRequestBuilder(this, TermVectorsAction.INSTANCE, index, type, id); } - @Deprecated - @Override - public ActionFuture termVector(final TermVectorsRequest request) { - return termVectors(request); - } - - @Deprecated - @Override - public void termVector(final TermVectorsRequest request, final ActionListener listener) { - termVectors(request, listener); - } - - @Deprecated - @Override - public TermVectorsRequestBuilder prepareTermVector() { - return prepareTermVectors(); - } - - @Deprecated - @Override - public TermVectorsRequestBuilder prepareTermVector(String index, String type, String id) { - return prepareTermVectors(index, type, id); - } - @Override public ActionFuture multiTermVectors(final MultiTermVectorsRequest request) { return execute(MultiTermVectorsAction.INSTANCE, request); diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestTermVectorsAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestTermVectorsAction.java index a312f6ab28409..89b8b9267f674 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestTermVectorsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestTermVectorsAction.java @@ -19,11 +19,9 @@ package org.elasticsearch.rest.action.document; -import org.apache.logging.log4j.LogManager; import org.elasticsearch.action.termvectors.TermVectorsRequest; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.VersionType; @@ -45,19 +43,13 @@ * TermVectorsRequest. */ public class RestTermVectorsAction extends BaseRestHandler { - private static final DeprecationLogger deprecationLogger = new DeprecationLogger( - LogManager.getLogger(RestTermVectorsAction.class)); public RestTermVectorsAction(Settings settings, RestController controller) { super(settings); - controller.registerWithDeprecatedHandler(GET, "/{index}/{type}/_termvectors", this, - GET, "/{index}/{type}/_termvector", deprecationLogger); - controller.registerWithDeprecatedHandler(POST, "/{index}/{type}/_termvectors", this, - POST, "/{index}/{type}/_termvector", deprecationLogger); - controller.registerWithDeprecatedHandler(GET, "/{index}/{type}/{id}/_termvectors", this, - GET, "/{index}/{type}/{id}/_termvector", deprecationLogger); - controller.registerWithDeprecatedHandler(POST, "/{index}/{type}/{id}/_termvectors", this, - POST, "/{index}/{type}/{id}/_termvector", deprecationLogger); + controller.registerHandler(GET, "/{index}/{type}/_termvectors", this); + controller.registerHandler(POST, "/{index}/{type}/_termvectors", this); + controller.registerHandler(GET, "/{index}/{type}/{id}/_termvectors", this); + controller.registerHandler(POST, "/{index}/{type}/{id}/_termvectors", this); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/termvectors/GetTermVectorsIT.java b/server/src/test/java/org/elasticsearch/action/termvectors/GetTermVectorsIT.java index a45012dc4b3de..442e27c0867b9 100644 --- a/server/src/test/java/org/elasticsearch/action/termvectors/GetTermVectorsIT.java +++ b/server/src/test/java/org/elasticsearch/action/termvectors/GetTermVectorsIT.java @@ -506,7 +506,7 @@ public void testDuelWithAndWithoutTermVectors() throws IOException, ExecutionExc for (int id = 0; id < content.length; id++) { Fields[] fields = new Fields[2]; for (int j = 0; j < indexNames.length; j++) { - TermVectorsResponse resp = client().prepareTermVector(indexNames[j], "type1", String.valueOf(id)) + TermVectorsResponse resp = client().prepareTermVectors(indexNames[j], "type1", String.valueOf(id)) .setOffsets(true) .setPositions(true) .setSelectedFields("field1") @@ -1069,7 +1069,7 @@ public void testWithKeywordAndNormalizer() throws IOException, ExecutionExceptio for (int id = 0; id < content.length; id++) { Fields[] fields = new Fields[2]; for (int j = 0; j < indexNames.length; j++) { - TermVectorsResponse resp = client().prepareTermVector(indexNames[j], "type1", String.valueOf(id)) + TermVectorsResponse resp = client().prepareTermVectors(indexNames[j], "type1", String.valueOf(id)) .setOffsets(true) .setPositions(true) .setSelectedFields("field1", "field2") diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestTermVectorsActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestTermVectorsActionTests.java deleted file mode 100644 index 88c867b0e56d1..0000000000000 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestTermVectorsActionTests.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * 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.rest.action.document; - -import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; -import org.elasticsearch.rest.RestChannel; -import org.elasticsearch.rest.RestController; -import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.RestRequest.Method; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.rest.FakeRestChannel; -import org.elasticsearch.test.rest.FakeRestRequest; -import org.elasticsearch.usage.UsageService; - -import java.util.Collections; - -import static org.mockito.Mockito.mock; - -public class RestTermVectorsActionTests extends ESTestCase { - private RestController controller; - - public void setUp() throws Exception { - super.setUp(); - controller = new RestController(Collections.emptySet(), null, - mock(NodeClient.class), - new NoneCircuitBreakerService(), - new UsageService()); - new RestTermVectorsAction(Settings.EMPTY, controller); - } - - public void testDeprecatedEndpoint() { - RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) - .withMethod(Method.POST) - .withPath("/some_index/some_type/some_id/_termvector") - .build(); - - performRequest(request); - assertWarnings("[POST /{index}/{type}/{id}/_termvector] is deprecated! Use" + - " [POST /{index}/{type}/{id}/_termvectors] instead."); - } - - private void performRequest(RestRequest request) { - RestChannel channel = new FakeRestChannel(request, false, 1); - ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - controller.dispatchRequest(request, channel, threadContext); - } -} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java index ed82808af7618..f1f3993261e11 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java @@ -492,13 +492,13 @@ private void assertUserExecutes(String user, String action, String index, boolea assertAccessIsAllowed("admin", "GET", "/" + index + "/_search"); assertAccessIsAllowed("admin", "GET", "/" + index + "/foo/1"); assertAccessIsAllowed(user, "GET", "/" + index + "/foo/1/_explain", "{ \"query\" : { \"match_all\" : {} } }"); - assertAccessIsAllowed(user, "GET", "/" + index + "/foo/1/_termvector"); + assertAccessIsAllowed(user, "GET", "/" + index + "/foo/1/_termvectors"); assertUserIsAllowed(user, "search", index); } else { assertAccessIsDenied(user, "GET", "/" + index + "/_count"); assertAccessIsDenied(user, "GET", "/" + index + "/_search"); assertAccessIsDenied(user, "GET", "/" + index + "/foo/1/_explain", "{ \"query\" : { \"match_all\" : {} } }"); - assertAccessIsDenied(user, "GET", "/" + index + "/foo/1/_termvector"); + assertAccessIsDenied(user, "GET", "/" + index + "/foo/1/_termvectors"); assertUserIsDenied(user, "search", index); } break; From 19b936dcceae7a57fc3940ec4c3e936675f9a3b5 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 3 Dec 2018 18:25:18 +0000 Subject: [PATCH 11/17] Fix broken links in painless docs (#36170) --- .../painless-contexts/painless-similarity-context.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/painless/painless-contexts/painless-similarity-context.asciidoc b/docs/painless/painless-contexts/painless-similarity-context.asciidoc index 9a8e59350d1a8..a8d66233e66cc 100644 --- a/docs/painless/painless-contexts/painless-similarity-context.asciidoc +++ b/docs/painless/painless-contexts/painless-similarity-context.asciidoc @@ -41,7 +41,7 @@ documents in a query. `doc.length` (`long`, read-only):: The number of tokens the current document has in the current field. This - is decoded from the stored {ref}/norms[norms] and may be approximate for + is decoded from the stored {ref}/norms.html[norms] and may be approximate for long fields `doc.freq` (`long`, read-only):: @@ -49,7 +49,7 @@ documents in a query. document for the current field. Note that the `query`, `field`, and `term` variables are also available to the -{ref}/painless-weight-context[weight context]. They are more efficiently used +{painless}/painless-weight-context.html[weight context]. They are more efficiently used there, as they are constant for all documents. For queries that contain multiple terms, the script is called once for each From fdec331b13dedf97d42b2e40d884f125a1668e4f Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 3 Dec 2018 10:55:31 -0800 Subject: [PATCH 12/17] [ILM] fix ilm.remove_policy rest-spec (#36165) The rest interface for remove-policy-from-index API does not support `_ilm/remove`, it requires that an `{index}` pattern be defined in the URL path. This fixes the rest-api-spec to reflect the implementation --- .../src/test/resources/rest-api-spec/api/ilm.remove_policy.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/api/ilm.remove_policy.json b/x-pack/plugin/src/test/resources/rest-api-spec/api/ilm.remove_policy.json index de3591d60269e..d9903ff8dc40d 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/api/ilm.remove_policy.json +++ b/x-pack/plugin/src/test/resources/rest-api-spec/api/ilm.remove_policy.json @@ -4,7 +4,7 @@ "methods": [ "POST" ], "url": { "path": "/{index}/_ilm/remove", - "paths": ["/{index}/_ilm/remove", "/_ilm/remove"], + "paths": ["/{index}/_ilm/remove"], "parts": { "index": { "type" : "string", From d27aa72b17f1ceda156f705a262ef8c5f4106c35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Mon, 3 Dec 2018 18:57:10 +0000 Subject: [PATCH 13/17] Added soft limit to open scroll contexts #25244 (#36009) This change adds a soft limit to open scroll contexts that can be controlled with the dynamic cluster setting `search.max_open_scroll_context` (defaults to 500). --- docs/reference/search/request/scroll.asciidoc | 5 ++ .../common/settings/ClusterSettings.java | 1 + .../elasticsearch/search/SearchService.java | 23 ++++++++ .../search/SearchServiceTests.java | 57 +++++++++++++++++++ 4 files changed, 86 insertions(+) diff --git a/docs/reference/search/request/scroll.asciidoc b/docs/reference/search/request/scroll.asciidoc index 4b96fe0e70678..f46a4a91e7f3c 100644 --- a/docs/reference/search/request/scroll.asciidoc +++ b/docs/reference/search/request/scroll.asciidoc @@ -125,6 +125,11 @@ TIP: Keeping older segments alive means that more file handles are needed. Ensure that you have configured your nodes to have ample free file handles. See <>. +NOTE: To prevent against issues caused by having too many scrolls open, the +user is not allowed to open scrolls past a certain limit. By default, the +maximum number of open scrolls is 500. This limit can be updated with the +`search.max_open_scroll_context` cluster setting. + You can check how many search contexts are open with the <>: diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 974c77210b533..621338f9c9814 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -387,6 +387,7 @@ public void apply(Settings value, Settings current, Settings previous) { SearchService.MAX_KEEPALIVE_SETTING, MultiBucketConsumerService.MAX_BUCKET_SETTING, SearchService.LOW_LEVEL_CANCELLATION_SETTING, + SearchService.MAX_OPEN_SCROLL_CONTEXT, Node.WRITE_PORTS_FILE_SETTING, Node.NODE_NAME_SETTING, Node.NODE_DATA_SETTING, diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 663214f49d8f4..98f2e1d2e7ecf 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -112,6 +112,7 @@ import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.function.LongSupplier; import java.util.function.Supplier; @@ -145,6 +146,9 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv public static final Setting DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS = Setting.boolSetting("search.default_allow_partial_results", true, Property.Dynamic, Property.NodeScope); + public static final Setting MAX_OPEN_SCROLL_CONTEXT = + Setting.intSetting("search.max_open_scroll_context", 500, 0, Property.Dynamic, Property.NodeScope); + private final ThreadPool threadPool; @@ -174,6 +178,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private volatile boolean lowLevelCancellation; + private volatile int maxOpenScrollContext; + private final Cancellable keepAliveReaper; private final AtomicLong idGenerator = new AtomicLong(); @@ -182,6 +188,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private final MultiBucketConsumerService multiBucketConsumerService; + private final AtomicInteger openScrollContexts = new AtomicInteger(); + public SearchService(ClusterService clusterService, IndicesService indicesService, ThreadPool threadPool, ScriptService scriptService, BigArrays bigArrays, FetchPhase fetchPhase, ResponseCollectorService responseCollectorService) { @@ -212,6 +220,8 @@ public SearchService(ClusterService clusterService, IndicesService indicesServic clusterService.getClusterSettings().addSettingsUpdateConsumer(DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS, this::setDefaultAllowPartialSearchResults); + maxOpenScrollContext = MAX_OPEN_SCROLL_CONTEXT.get(settings); + clusterService.getClusterSettings().addSettingsUpdateConsumer(MAX_OPEN_SCROLL_CONTEXT, this::setMaxOpenScrollContext); lowLevelCancellation = LOW_LEVEL_CANCELLATION_SETTING.get(settings); clusterService.getClusterSettings().addSettingsUpdateConsumer(LOW_LEVEL_CANCELLATION_SETTING, this::setLowLevelCancellation); @@ -243,6 +253,10 @@ public boolean defaultAllowPartialSearchResults() { return defaultAllowPartialSearchResults; } + private void setMaxOpenScrollContext(int maxOpenScrollContext) { + this.maxOpenScrollContext = maxOpenScrollContext; + } + private void setLowLevelCancellation(Boolean lowLevelCancellation) { this.lowLevelCancellation = lowLevelCancellation; } @@ -592,11 +606,19 @@ private SearchContext findContext(long id, TransportRequest request) throws Sear } final SearchContext createAndPutContext(ShardSearchRequest request) throws IOException { + if (request.scroll() != null && openScrollContexts.get() >= maxOpenScrollContext) { + throw new ElasticsearchException( + "Trying to create too many scroll contexts. Must be less than or equal to: [" + + maxOpenScrollContext + "]. " + "This limit can be set by changing the [" + + MAX_OPEN_SCROLL_CONTEXT.getKey() + "] setting."); + } + SearchContext context = createContext(request); boolean success = false; try { putContext(context); if (request.scroll() != null) { + openScrollContexts.incrementAndGet(); context.indexShard().getSearchOperationListener().onNewScrollContext(context); } context.indexShard().getSearchOperationListener().onNewContext(context); @@ -696,6 +718,7 @@ public boolean freeContext(long id) { assert context.refCount() > 0 : " refCount must be > 0: " + context.refCount(); context.indexShard().getSearchOperationListener().onFreeContext(context); if (context.scrollContext() != null) { + openScrollContexts.decrementAndGet(); context.indexShard().getSearchOperationListener().onFreeScrollContext(context); } return true; diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index 45adc1149a3eb..30598311ad574 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -21,12 +21,14 @@ import com.carrotsearch.hppc.IntArrayList; import org.apache.lucene.search.Query; import org.apache.lucene.store.AlreadyClosedException; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.action.search.ClearScrollRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.WriteRequest; @@ -76,6 +78,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.LinkedList; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; @@ -417,6 +420,44 @@ searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_A } } + /** + * test that creating more than the allowed number of scroll contexts throws an exception + */ + public void testMaxOpenScrollContexts() throws RuntimeException { + createIndex("index"); + client().prepareIndex("index", "type", "1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); + + final SearchService service = getInstanceFromNode(SearchService.class); + final IndicesService indicesService = getInstanceFromNode(IndicesService.class); + final IndexService indexService = indicesService.indexServiceSafe(resolveIndex("index")); + final IndexShard indexShard = indexService.getShard(0); + + // Open all possible scrolls, clear some of them, then open more until the limit is reached + LinkedList clearScrollIds = new LinkedList<>(); + + for (int i = 0; i < SearchService.MAX_OPEN_SCROLL_CONTEXT.get(Settings.EMPTY); i++) { + SearchResponse searchResponse = client().prepareSearch("index").setSize(1).setScroll("1m").get(); + + if (randomInt(4) == 0) clearScrollIds.addLast(searchResponse.getScrollId()); + } + + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + clearScrollRequest.setScrollIds(clearScrollIds); + client().clearScroll(clearScrollRequest); + + for (int i = 0; i < clearScrollIds.size(); i++) { + client().prepareSearch("index").setSize(1).setScroll("1m").get(); + } + + ElasticsearchException ex = expectThrows(ElasticsearchException.class, + () -> service.createAndPutContext(new ShardScrollRequestTest(indexShard.shardId()))); + assertEquals( + "Trying to create too many scroll contexts. Must be less than or equal to: [" + + SearchService.MAX_OPEN_SCROLL_CONTEXT.get(Settings.EMPTY) + "]. " + + "This limit can be set by changing the [search.max_open_scroll_context] setting.", + ex.getMessage()); + } + public static class FailOnRewriteQueryPlugin extends Plugin implements SearchPlugin { @Override public List> getQueries() { @@ -472,6 +513,22 @@ public String getWriteableName() { } } + public static class ShardScrollRequestTest extends ShardSearchLocalRequest { + private Scroll scroll; + + ShardScrollRequestTest(ShardId shardId) { + super(shardId, 1, SearchType.DEFAULT, new SearchSourceBuilder(), + new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, true, null, null); + + this.scroll = new Scroll(TimeValue.timeValueMinutes(1)); + } + + @Override + public Scroll scroll() { + return this.scroll; + } + } + public void testCanMatch() throws IOException { createIndex("index"); final SearchService service = getInstanceFromNode(SearchService.class); From e082cda0d65041dabf6c43a0c17ed5cab693e4b4 Mon Sep 17 00:00:00 2001 From: lcawl Date: Mon, 3 Dec 2018 12:17:26 -0800 Subject: [PATCH 14/17] [DOCS] Fixes peer link --- .../painless-contexts/painless-similarity-context.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/painless/painless-contexts/painless-similarity-context.asciidoc b/docs/painless/painless-contexts/painless-similarity-context.asciidoc index a8d66233e66cc..58609ce705e10 100644 --- a/docs/painless/painless-contexts/painless-similarity-context.asciidoc +++ b/docs/painless/painless-contexts/painless-similarity-context.asciidoc @@ -49,7 +49,7 @@ documents in a query. document for the current field. Note that the `query`, `field`, and `term` variables are also available to the -{painless}/painless-weight-context.html[weight context]. They are more efficiently used +<>. They are more efficiently used there, as they are constant for all documents. For queries that contain multiple terms, the script is called once for each From 05d52b222f354f53d705a71beadc70f1bad9d1e3 Mon Sep 17 00:00:00 2001 From: lcawl Date: Mon, 3 Dec 2018 12:38:53 -0800 Subject: [PATCH 15/17] [DOCs] More broken painless links --- .../painless-contexts/painless-similarity-context.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/painless/painless-contexts/painless-similarity-context.asciidoc b/docs/painless/painless-contexts/painless-similarity-context.asciidoc index 58609ce705e10..1d847f516c8be 100644 --- a/docs/painless/painless-contexts/painless-similarity-context.asciidoc +++ b/docs/painless/painless-contexts/painless-similarity-context.asciidoc @@ -16,7 +16,7 @@ documents in a query. User-defined parameters passed in at query-time. `weight` (`float`, read-only):: - The weight as calculated by a {ref}/painless-weight-context[weight script] + The weight as calculated by a <> `query.boost` (`float`, read-only):: The boost value if provided by the query. If this is not provided the From a6ff9f7f66dcb4648a9d36cfd70bb6c7f495ea9c Mon Sep 17 00:00:00 2001 From: Andy Bristol Date: Mon, 3 Dec 2018 15:08:00 -0800 Subject: [PATCH 16/17] [test] generate unique user names (#36179) --- .../xpack/sql/qa/security/UserFunctionIT.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java index 4ba3875cac016..1538f5302d6bb 100644 --- a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java +++ b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java @@ -59,11 +59,9 @@ protected String getProtocol() { private void setUpUsers() throws IOException { int usersCount = name.getMethodName().startsWith("testSingle") ? 1 : randomIntBetween(5, 15); users = new ArrayList(usersCount); - - for(int i = 0; i < usersCount; i++) { - String randomUserName = randomAlphaOfLengthBetween(1, 15); - users.add(randomUserName); - createUser(randomUserName, MINIMAL_ACCESS_ROLE); + users.addAll(randomUnique(() -> randomAlphaOfLengthBetween(1, 15), usersCount)); + for (String user : users) { + createUser(user, MINIMAL_ACCESS_ROLE); } } From 01b8f99c17c4fcbe5d6ab9c3bca9153f21e610ee Mon Sep 17 00:00:00 2001 From: Andy Bristol Date: Mon, 3 Dec 2018 16:07:16 -0800 Subject: [PATCH 17/17] [test] mute RemoveCorruptedShardDataCommandIT --- .../index/shard/RemoveCorruptedShardDataCommandIT.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandIT.java b/server/src/test/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandIT.java index 26f04319a25b6..4e094b8e29192 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandIT.java +++ b/server/src/test/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandIT.java @@ -28,6 +28,7 @@ import org.apache.lucene.store.Lock; import org.apache.lucene.store.LockObtainFailedException; import org.apache.lucene.store.NativeFSLockFactory; +import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplanation; import org.elasticsearch.action.admin.cluster.node.stats.NodeStats; import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsResponse; @@ -98,6 +99,7 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.startsWith; +@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/36189") @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, numDataNodes = 0) public class RemoveCorruptedShardDataCommandIT extends ESIntegTestCase {