From 7c56cc26247f9547def09ccd8351e108e21f9387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 10 Apr 2018 10:14:52 +0200 Subject: [PATCH 01/11] Make ranking evaluation details accessible for client Allow high level java rest client to access details of the metric calculation by making them accessible across packages. Also renaming the inner `Breakdown` classes of the evaluation metrics to `Detail` to better communicate their use. --- .../index/rankeval/MeanReciprocalRank.java | 22 ++++++------ .../index/rankeval/PrecisionAtK.java | 24 ++++++------- .../RankEvalNamedXContentProvider.java | 4 +-- .../index/rankeval/RankEvalPlugin.java | 4 +-- .../index/rankeval/EvalQueryQualityTests.java | 6 ++-- .../rankeval/MeanReciprocalRankTests.java | 8 ++--- .../index/rankeval/PrecisionAtKTests.java | 36 +++++++++---------- .../index/rankeval/RankEvalRequestIT.java | 12 +++---- 8 files changed, 58 insertions(+), 58 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/MeanReciprocalRank.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/MeanReciprocalRank.java index 0f51f6d5d6369..eb20dc8c680f9 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/MeanReciprocalRank.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/MeanReciprocalRank.java @@ -128,7 +128,7 @@ public EvalQueryQuality evaluate(String taskId, SearchHit[] hits, double reciprocalRank = (firstRelevant == -1) ? 0 : 1.0d / firstRelevant; EvalQueryQuality evalQueryQuality = new EvalQueryQuality(taskId, reciprocalRank); - evalQueryQuality.setMetricDetails(new Breakdown(firstRelevant)); + evalQueryQuality.setMetricDetails(new Detail(firstRelevant)); evalQueryQuality.addHitsAndRatings(ratedHits); return evalQueryQuality; } @@ -181,16 +181,16 @@ public final int hashCode() { return Objects.hash(relevantRatingThreshhold, k); } - static class Breakdown implements MetricDetail { + public static final class Detail implements MetricDetail { private final int firstRelevantRank; private static ParseField FIRST_RELEVANT_RANK_FIELD = new ParseField("first_relevant"); - Breakdown(int firstRelevantRank) { + Detail(int firstRelevantRank) { this.firstRelevantRank = firstRelevantRank; } - Breakdown(StreamInput in) throws IOException { + Detail(StreamInput in) throws IOException { this.firstRelevantRank = in.readVInt(); } @@ -206,15 +206,15 @@ public XContentBuilder innerToXContent(XContentBuilder builder, Params params) return builder.field(FIRST_RELEVANT_RANK_FIELD.getPreferredName(), firstRelevantRank); } - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, true, args -> { - return new Breakdown((Integer) args[0]); + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, true, args -> { + return new Detail((Integer) args[0]); }); static { PARSER.declareInt(constructorArg(), FIRST_RELEVANT_RANK_FIELD); } - public static Breakdown fromXContent(XContentParser parser) { + public static Detail fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } @@ -232,24 +232,24 @@ public String getWriteableName() { * the ranking of the first relevant document, or -1 if no relevant document was * found */ - int getFirstRelevantRank() { + public int getFirstRelevantRank() { return firstRelevantRank; } @Override - public final boolean equals(Object obj) { + public boolean equals(Object obj) { if (this == obj) { return true; } if (obj == null || getClass() != obj.getClass()) { return false; } - MeanReciprocalRank.Breakdown other = (MeanReciprocalRank.Breakdown) obj; + MeanReciprocalRank.Detail other = (MeanReciprocalRank.Detail) obj; return Objects.equals(firstRelevantRank, other.firstRelevantRank); } @Override - public final int hashCode() { + public int hashCode() { return Objects.hash(firstRelevantRank); } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtK.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtK.java index 15d955935eeff..136158ea5cba7 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtK.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtK.java @@ -181,7 +181,7 @@ public EvalQueryQuality evaluate(String taskId, SearchHit[] hits, } EvalQueryQuality evalQueryQuality = new EvalQueryQuality(taskId, precision); evalQueryQuality.setMetricDetails( - new PrecisionAtK.Breakdown(truePositives, truePositives + falsePositives)); + new PrecisionAtK.Detail(truePositives, truePositives + falsePositives)); evalQueryQuality.addHitsAndRatings(ratedSearchHits); return evalQueryQuality; } @@ -217,19 +217,19 @@ public final int hashCode() { return Objects.hash(relevantRatingThreshhold, ignoreUnlabeled, k); } - static class Breakdown implements MetricDetail { + public static final class Detail implements MetricDetail { private static final ParseField DOCS_RETRIEVED_FIELD = new ParseField("docs_retrieved"); private static final ParseField RELEVANT_DOCS_RETRIEVED_FIELD = new ParseField("relevant_docs_retrieved"); private int relevantRetrieved; private int retrieved; - Breakdown(int relevantRetrieved, int retrieved) { + Detail(int relevantRetrieved, int retrieved) { this.relevantRetrieved = relevantRetrieved; this.retrieved = retrieved; } - Breakdown(StreamInput in) throws IOException { + Detail(StreamInput in) throws IOException { this.relevantRetrieved = in.readVInt(); this.retrieved = in.readVInt(); } @@ -242,8 +242,8 @@ public XContentBuilder innerToXContent(XContentBuilder builder, Params params) return builder; } - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, true, args -> { - return new Breakdown((Integer) args[0], (Integer) args[1]); + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, true, args -> { + return new Detail((Integer) args[0], (Integer) args[1]); }); static { @@ -251,7 +251,7 @@ public XContentBuilder innerToXContent(XContentBuilder builder, Params params) PARSER.declareInt(constructorArg(), DOCS_RETRIEVED_FIELD); } - public static Breakdown fromXContent(XContentParser parser) { + public static Detail fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } @@ -266,29 +266,29 @@ public String getWriteableName() { return NAME; } - int getRelevantRetrieved() { + public int getRelevantRetrieved() { return relevantRetrieved; } - int getRetrieved() { + public int getRetrieved() { return retrieved; } @Override - public final boolean equals(Object obj) { + public boolean equals(Object obj) { if (this == obj) { return true; } if (obj == null || getClass() != obj.getClass()) { return false; } - PrecisionAtK.Breakdown other = (PrecisionAtK.Breakdown) obj; + PrecisionAtK.Detail other = (PrecisionAtK.Detail) obj; return Objects.equals(relevantRetrieved, other.relevantRetrieved) && Objects.equals(retrieved, other.retrieved); } @Override - public final int hashCode() { + public int hashCode() { return Objects.hash(relevantRetrieved, retrieved); } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalNamedXContentProvider.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalNamedXContentProvider.java index 54d68774a016e..c5785ca3847d4 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalNamedXContentProvider.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalNamedXContentProvider.java @@ -38,9 +38,9 @@ public List getNamedXContentParsers() { namedXContent.add(new NamedXContentRegistry.Entry(EvaluationMetric.class, new ParseField(DiscountedCumulativeGain.NAME), DiscountedCumulativeGain::fromXContent)); namedXContent.add(new NamedXContentRegistry.Entry(MetricDetail.class, new ParseField(PrecisionAtK.NAME), - PrecisionAtK.Breakdown::fromXContent)); + PrecisionAtK.Detail::fromXContent)); namedXContent.add(new NamedXContentRegistry.Entry(MetricDetail.class, new ParseField(MeanReciprocalRank.NAME), - MeanReciprocalRank.Breakdown::fromXContent)); + MeanReciprocalRank.Detail::fromXContent)); return namedXContent; } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalPlugin.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalPlugin.java index d4ccd7c2180fe..884cf3bafdcda 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalPlugin.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalPlugin.java @@ -60,9 +60,9 @@ public List getNamedWriteables() { namedWriteables.add(new NamedWriteableRegistry.Entry(EvaluationMetric.class, MeanReciprocalRank.NAME, MeanReciprocalRank::new)); namedWriteables.add( new NamedWriteableRegistry.Entry(EvaluationMetric.class, DiscountedCumulativeGain.NAME, DiscountedCumulativeGain::new)); - namedWriteables.add(new NamedWriteableRegistry.Entry(MetricDetail.class, PrecisionAtK.NAME, PrecisionAtK.Breakdown::new)); + namedWriteables.add(new NamedWriteableRegistry.Entry(MetricDetail.class, PrecisionAtK.NAME, PrecisionAtK.Detail::new)); namedWriteables - .add(new NamedWriteableRegistry.Entry(MetricDetail.class, MeanReciprocalRank.NAME, MeanReciprocalRank.Breakdown::new)); + .add(new NamedWriteableRegistry.Entry(MetricDetail.class, MeanReciprocalRank.NAME, MeanReciprocalRank.Detail::new)); return namedWriteables; } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/EvalQueryQualityTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/EvalQueryQualityTests.java index df6de75ba2cb4..112cf4eaaf72e 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/EvalQueryQualityTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/EvalQueryQualityTests.java @@ -69,9 +69,9 @@ public static EvalQueryQuality randomEvalQueryQuality() { randomDoubleBetween(0.0, 1.0, true)); if (randomBoolean()) { if (randomBoolean()) { - evalQueryQuality.setMetricDetails(new PrecisionAtK.Breakdown(randomIntBetween(0, 1000), randomIntBetween(0, 1000))); + evalQueryQuality.setMetricDetails(new PrecisionAtK.Detail(randomIntBetween(0, 1000), randomIntBetween(0, 1000))); } else { - evalQueryQuality.setMetricDetails(new MeanReciprocalRank.Breakdown(randomIntBetween(0, 1000))); + evalQueryQuality.setMetricDetails(new MeanReciprocalRank.Detail(randomIntBetween(0, 1000))); } } evalQueryQuality.addHitsAndRatings(ratedHits); @@ -137,7 +137,7 @@ private static EvalQueryQuality mutateTestItem(EvalQueryQuality original) { break; case 2: if (metricDetails == null) { - metricDetails = new PrecisionAtK.Breakdown(1, 5); + metricDetails = new PrecisionAtK.Detail(1, 5); } else { metricDetails = null; } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/MeanReciprocalRankTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/MeanReciprocalRankTests.java index 0ac9cb0d901e6..c9ff39bbd118a 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/MeanReciprocalRankTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/MeanReciprocalRankTests.java @@ -96,7 +96,7 @@ public void testMaxAcceptableRank() { int rankAtFirstRelevant = relevantAt + 1; EvalQueryQuality evaluation = reciprocalRank.evaluate("id", hits, ratedDocs); assertEquals(1.0 / rankAtFirstRelevant, evaluation.getQualityLevel(), Double.MIN_VALUE); - assertEquals(rankAtFirstRelevant, ((MeanReciprocalRank.Breakdown) evaluation.getMetricDetails()).getFirstRelevantRank()); + assertEquals(rankAtFirstRelevant, ((MeanReciprocalRank.Detail) evaluation.getMetricDetails()).getFirstRelevantRank()); // check that if we have fewer search hits than relevant doc position, // we don't find any result and get 0.0 quality level @@ -121,7 +121,7 @@ public void testEvaluationOneRelevantInResults() { EvalQueryQuality evaluation = reciprocalRank.evaluate("id", hits, ratedDocs); assertEquals(1.0 / (relevantAt + 1), evaluation.getQualityLevel(), Double.MIN_VALUE); - assertEquals(relevantAt + 1, ((MeanReciprocalRank.Breakdown) evaluation.getMetricDetails()).getFirstRelevantRank()); + assertEquals(relevantAt + 1, ((MeanReciprocalRank.Detail) evaluation.getMetricDetails()).getFirstRelevantRank()); } /** @@ -141,7 +141,7 @@ public void testPrecisionAtFiveRelevanceThreshold() { MeanReciprocalRank reciprocalRank = new MeanReciprocalRank(2, 10); EvalQueryQuality evaluation = reciprocalRank.evaluate("id", hits, rated); assertEquals((double) 1 / 3, evaluation.getQualityLevel(), 0.00001); - assertEquals(3, ((MeanReciprocalRank.Breakdown) evaluation.getMetricDetails()).getFirstRelevantRank()); + assertEquals(3, ((MeanReciprocalRank.Detail) evaluation.getMetricDetails()).getFirstRelevantRank()); } public void testCombine() { @@ -165,7 +165,7 @@ public void testNoResults() throws Exception { SearchHit[] hits = new SearchHit[0]; EvalQueryQuality evaluated = (new MeanReciprocalRank()).evaluate("id", hits, Collections.emptyList()); assertEquals(0.0d, evaluated.getQualityLevel(), 0.00001); - assertEquals(-1, ((MeanReciprocalRank.Breakdown) evaluated.getMetricDetails()).getFirstRelevantRank()); + assertEquals(-1, ((MeanReciprocalRank.Detail) evaluated.getMetricDetails()).getFirstRelevantRank()); } public void testXContentRoundtrip() throws IOException { diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtKTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtKTests.java index bf5bae4c334a9..3efff57920b84 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtKTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtKTests.java @@ -54,8 +54,8 @@ public void testPrecisionAtFiveCalculation() { rated.add(createRatedDoc("test", "0", RELEVANT_RATING_1)); EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", toSearchHits(rated, "test"), rated); assertEquals(1, evaluated.getQualityLevel(), 0.00001); - assertEquals(1, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(1, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(1, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(1, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); } public void testPrecisionAtFiveIgnoreOneResult() { @@ -67,8 +67,8 @@ public void testPrecisionAtFiveIgnoreOneResult() { rated.add(createRatedDoc("test", "4", IRRELEVANT_RATING_0)); EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", toSearchHits(rated, "test"), rated); assertEquals((double) 4 / 5, evaluated.getQualityLevel(), 0.00001); - assertEquals(4, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(5, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(4, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(5, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); } /** @@ -86,8 +86,8 @@ public void testPrecisionAtFiveRelevanceThreshold() { PrecisionAtK precisionAtN = new PrecisionAtK(2, false, 5); EvalQueryQuality evaluated = precisionAtN.evaluate("id", toSearchHits(rated, "test"), rated); assertEquals((double) 3 / 5, evaluated.getQualityLevel(), 0.00001); - assertEquals(3, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(5, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(3, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(5, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); } public void testPrecisionAtFiveCorrectIndex() { @@ -100,8 +100,8 @@ public void testPrecisionAtFiveCorrectIndex() { // the following search hits contain only the last three documents EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", toSearchHits(rated.subList(2, 5), "test"), rated); assertEquals((double) 2 / 3, evaluated.getQualityLevel(), 0.00001); - assertEquals(2, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(3, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(2, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(3, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); } public void testIgnoreUnlabeled() { @@ -115,15 +115,15 @@ public void testIgnoreUnlabeled() { EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", searchHits, rated); assertEquals((double) 2 / 3, evaluated.getQualityLevel(), 0.00001); - assertEquals(2, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(3, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(2, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(3, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); // also try with setting `ignore_unlabeled` PrecisionAtK prec = new PrecisionAtK(1, true, 10); evaluated = prec.evaluate("id", searchHits, rated); assertEquals((double) 2 / 2, evaluated.getQualityLevel(), 0.00001); - assertEquals(2, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(2, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(2, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(2, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); } public void testNoRatedDocs() throws Exception { @@ -134,23 +134,23 @@ public void testNoRatedDocs() throws Exception { } EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", hits, Collections.emptyList()); assertEquals(0.0d, evaluated.getQualityLevel(), 0.00001); - assertEquals(0, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(5, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(5, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); // also try with setting `ignore_unlabeled` PrecisionAtK prec = new PrecisionAtK(1, true, 10); evaluated = prec.evaluate("id", hits, Collections.emptyList()); assertEquals(0.0d, evaluated.getQualityLevel(), 0.00001); - assertEquals(0, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(0, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); } public void testNoResults() throws Exception { SearchHit[] hits = new SearchHit[0]; EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", hits, Collections.emptyList()); assertEquals(0.0d, evaluated.getQualityLevel(), 0.00001); - assertEquals(0, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRelevantRetrieved()); - assertEquals(0, ((PrecisionAtK.Breakdown) evaluated.getMetricDetails()).getRetrieved()); + assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved()); + assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved()); } public void testParseFromXContent() throws IOException { diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java index f0242e2bccdb6..8a3bad50b22da 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java @@ -25,7 +25,7 @@ import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilders; -import org.elasticsearch.index.rankeval.PrecisionAtK.Breakdown; +import org.elasticsearch.index.rankeval.PrecisionAtK.Detail; import org.elasticsearch.indices.IndexClosedException; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.search.builder.SearchSourceBuilder; @@ -271,7 +271,7 @@ public void testIndicesOptions() { request.setRankEvalSpec(task); RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, request).actionGet(); - Breakdown details = (PrecisionAtK.Breakdown) response.getPartialResults().get("amsterdam_query").getMetricDetails(); + Detail details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails(); assertEquals(7, details.getRetrieved()); assertEquals(6, details.getRelevantRetrieved()); @@ -280,7 +280,7 @@ public void testIndicesOptions() { request.indicesOptions(IndicesOptions.fromParameters(null, "true", null, SearchRequest.DEFAULT_INDICES_OPTIONS)); response = client().execute(RankEvalAction.INSTANCE, request).actionGet(); - details = (PrecisionAtK.Breakdown) response.getPartialResults().get("amsterdam_query").getMetricDetails(); + details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails(); assertEquals(6, details.getRetrieved()); assertEquals(5, details.getRelevantRetrieved()); @@ -295,12 +295,12 @@ public void testIndicesOptions() { request = new RankEvalRequest(task, new String[] { "tes*" }); request.indicesOptions(IndicesOptions.fromParameters("none", null, null, SearchRequest.DEFAULT_INDICES_OPTIONS)); response = client().execute(RankEvalAction.INSTANCE, request).actionGet(); - details = (PrecisionAtK.Breakdown) response.getPartialResults().get("amsterdam_query").getMetricDetails(); + details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails(); assertEquals(0, details.getRetrieved()); request.indicesOptions(IndicesOptions.fromParameters("open", null, null, SearchRequest.DEFAULT_INDICES_OPTIONS)); response = client().execute(RankEvalAction.INSTANCE, request).actionGet(); - details = (PrecisionAtK.Breakdown) response.getPartialResults().get("amsterdam_query").getMetricDetails(); + details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails(); assertEquals(6, details.getRetrieved()); assertEquals(5, details.getRelevantRetrieved()); @@ -313,7 +313,7 @@ public void testIndicesOptions() { request = new RankEvalRequest(task, new String[] { "bad*" }); request.indicesOptions(IndicesOptions.fromParameters(null, null, "true", SearchRequest.DEFAULT_INDICES_OPTIONS)); response = client().execute(RankEvalAction.INSTANCE, request).actionGet(); - details = (PrecisionAtK.Breakdown) response.getPartialResults().get("amsterdam_query").getMetricDetails(); + details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails(); assertEquals(0, details.getRetrieved()); request.indicesOptions(IndicesOptions.fromParameters(null, null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS)); From 124fecd22103f45aab7a4003c462d93fd5fa9a9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 10 Apr 2018 10:15:31 +0200 Subject: [PATCH 02/11] [Docs] Add rankEval method for Jva HL client This change adds documentation about using the ranking evaluation API from the high level Java REST client. Closes #28694 --- .../documentation/SearchDocumentationIT.java | 84 +++++++++++++++++ .../high-level/search/rank-eval.asciidoc | 89 +++++++++++++++++++ .../high-level/supported-apis.asciidoc | 2 + 3 files changed, 175 insertions(+) create mode 100644 docs/java-rest/high-level/search/rank-eval.asciidoc diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java index bd1cf48f14195..a8dde67fa94a0 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java @@ -37,6 +37,7 @@ import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.client.ESRestHighLevelClientTestCase; import org.elasticsearch.client.RestHighLevelClient; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.text.Text; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.unit.TimeValue; @@ -44,6 +45,16 @@ import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.rankeval.EvalQueryQuality; +import org.elasticsearch.index.rankeval.EvaluationMetric; +import org.elasticsearch.index.rankeval.MetricDetail; +import org.elasticsearch.index.rankeval.PrecisionAtK; +import org.elasticsearch.index.rankeval.RankEvalRequest; +import org.elasticsearch.index.rankeval.RankEvalResponse; +import org.elasticsearch.index.rankeval.RankEvalSpec; +import org.elasticsearch.index.rankeval.RatedDocument; +import org.elasticsearch.index.rankeval.RatedRequest; +import org.elasticsearch.index.rankeval.RatedSearchHit; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.Scroll; import org.elasticsearch.search.SearchHit; @@ -74,6 +85,7 @@ import org.elasticsearch.search.suggest.term.TermSuggestion; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -688,6 +700,78 @@ public void onFailure(Exception e) { } } + public void testRankEval() throws Exception { + indexSearchTestData(); + RestHighLevelClient client = highLevelClient(); + { + // tag::rank-eval-request-basic + EvaluationMetric metric = new PrecisionAtK(); // <1> + List ratedDocs = new ArrayList<>(); + ratedDocs.add(new RatedDocument("posts", "1", 1)); // <2> + SearchSourceBuilder searchQuery = new SearchSourceBuilder(); + searchQuery.query(QueryBuilders.matchQuery("user", "kimchy"));// <3> + RatedRequest ratedRequest = // <4> + new RatedRequest("kimchy_query", ratedDocs, searchQuery); + List ratedRequests = Arrays.asList(ratedRequest); + RankEvalSpec specification = + new RankEvalSpec(ratedRequests, metric); // <5> + RankEvalRequest request = // <6> + new RankEvalRequest(specification, new String[] { "posts" }); + // end::rank-eval-request-basic + + // tag::rank-eval-execute + RankEvalResponse response = client.rankEval(request); + // end::rank-eval-execute + + logger.warn(Strings.toString(response)); + // tag::rank-eval-response + double evaluationResult = response.getEvaluationResult(); // <1> + assertEquals(1.0 / 3.0, evaluationResult, 0.0); + Map partialResults = + response.getPartialResults(); + EvalQueryQuality evalQuality = + partialResults.get("kimchy_query"); // <2> + assertEquals("kimchy_query", evalQuality.getId()); + double qualityLevel = evalQuality.getQualityLevel(); // <3> + assertEquals(1.0 / 3.0, qualityLevel, 0.0); + List hitsAndRatings = evalQuality.getHitsAndRatings(); + RatedSearchHit ratedSearchHit = hitsAndRatings.get(0); + assertEquals("3", ratedSearchHit.getSearchHit().getId()); // <4> + assertFalse(ratedSearchHit.getRating().isPresent()); // <5> + MetricDetail metricDetails = evalQuality.getMetricDetails(); + String metricName = metricDetails.getMetricName(); + assertEquals(PrecisionAtK.NAME, metricName); // <6> + PrecisionAtK.Detail detail = (PrecisionAtK.Detail) metricDetails; + assertEquals(1, detail.getRelevantRetrieved()); // <7> + assertEquals(3, detail.getRetrieved()); + // end::rank-eval-response + + // tag::rank-eval-execute-listener + ActionListener listener = new ActionListener() { + @Override + public void onResponse(RankEvalResponse response) { + // <1> + } + + @Override + public void onFailure(Exception e) { + // <2> + } + }; + // end::rank-eval-execute-listener + + // Replace the empty listener by a blocking listener in test + final CountDownLatch latch = new CountDownLatch(1); + listener = new LatchedActionListener<>(listener, latch); + + // tag::rank-eval-execute-async + client.rankEvalAsync(request, listener); // <1> + // end::rank-eval-execute-async + + assertTrue(latch.await(30L, TimeUnit.SECONDS)); + } + } + public void testMultiSearch() throws Exception { indexSearchTestData(); RestHighLevelClient client = highLevelClient(); diff --git a/docs/java-rest/high-level/search/rank-eval.asciidoc b/docs/java-rest/high-level/search/rank-eval.asciidoc new file mode 100644 index 0000000000000..6db0dadd00ed7 --- /dev/null +++ b/docs/java-rest/high-level/search/rank-eval.asciidoc @@ -0,0 +1,89 @@ +[[java-rest-high-rank-eval]] +=== Ranking Evaluation API + +The `rankEval` method allows to evaluate the quality of ranked search +results over a set of search request. Given sets of manually rated +documents for each search request, ranking evaluation performs a +<> request and calculates +information retrieval metrics like _mean reciprocal rank_, _precision_ +or _discounted cumulative gain_ on the returned results. + +[[java-rest-high-rank-eval-request]] +==== Ranking Evaluation Request + +In order to build a `RankEvalRequest`, you first need to create an +evaluation specification (`RankEvalSpec`). This specification requires +to define the evaluation metric that is going to be calculated, as well +as a list of rated documents per search requests. Creating the ranking +evaluation request then takes the specification and a list of target +indices as arguments: + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/SearchDocumentationIT.java[rank-eval-request-basic] +-------------------------------------------------- +<1> Define the metric used in the evaluation +<2> Add rated documents, specified by index name, id and rating +<3> Create the search query to evaluate +<4> Combine the three former parts into a `RatedRequest` +<5> Create the ranking evaluation specification +<6> Create the ranking evaluation request + +[[java-rest-high-rank-eval-sync]] +==== Synchronous Execution + +The `rankEval` method executes `RankEvalRequest`s synchronously: + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/SearchDocumentationIT.java[rank-eval-execute] +-------------------------------------------------- + +[[java-rest-high-rank-eval-async]] +==== Asynchronous Execution + +The `rankEvalAsync` method executes `RankEvalRequest`s asynchronously, +calling the provided `ActionListener` when the response is ready. + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/SearchDocumentationIT.java[rank-eval-execute-async] +-------------------------------------------------- +<1> The `RankEvalRequest` to execute and the `ActionListener` to use when +the execution completes + +The asynchronous method does not block and returns immediately. Once it is +completed the `ActionListener` is called back using the `onResponse` method +if the execution successfully completed or using the `onFailure` method if +it failed. + +A typical listener for `RankEvalResponse` looks like: + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/SearchDocumentationIT.java[rank-eval-execute-listener] +-------------------------------------------------- +<1> Called when the execution is successfully completed. +<2> Called when the whole `RankEvalRequest` fails. + +==== RankEvalResponse + +The `RankEvalResponse` that is returned by executing the request +contains information about the overall evaluation score, the +scores of each individual search request in the set of queries and +detailed information about search hits and details about the metric +calculation per partial result. + +["source","java",subs="attributes,callouts,macros"] +-------------------------------------------------- +include-tagged::{doc-tests}/SearchDocumentationIT.java[rank-eval-response] +-------------------------------------------------- +<1> The overall evaluation result +<2> Partial results that are keyed by their query id +<3> The metric score for each partial result +<4> Rated search hits contain a fully fledged `SearchHit` +<5> Rated search hits also contain an `Optional` rating that +is not present if the document did not get a rating in the request +<6> Metric details are named after the metric used in the request +<7> After casting to the metric used in the request, the +metric details offers insight into parts of the metric calculation \ No newline at end of file diff --git a/docs/java-rest/high-level/supported-apis.asciidoc b/docs/java-rest/high-level/supported-apis.asciidoc index 29052171cddc6..1f3d7a3744300 100644 --- a/docs/java-rest/high-level/supported-apis.asciidoc +++ b/docs/java-rest/high-level/supported-apis.asciidoc @@ -32,10 +32,12 @@ The Java High Level REST Client supports the following Search APIs: * <> * <> * <> +* <> include::search/search.asciidoc[] include::search/scroll.asciidoc[] include::search/multi-search.asciidoc[] +include::search/rank-eval.asciidoc[] == Miscellaneous APIs From 6e62b481b449103b634834da4e3f68639bf1fa6f Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Thu, 19 Apr 2018 15:09:14 +0200 Subject: [PATCH 03/11] Update plan for the removal of mapping types. (#29586) 8.x will no longer allow types in APIs and 7.x will issue deprecation warnings when `include_type_name` is set to `false`. --- docs/reference/mapping/removal_of_types.asciidoc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/reference/mapping/removal_of_types.asciidoc b/docs/reference/mapping/removal_of_types.asciidoc index 60776763ea844..95881ba83856f 100644 --- a/docs/reference/mapping/removal_of_types.asciidoc +++ b/docs/reference/mapping/removal_of_types.asciidoc @@ -258,15 +258,17 @@ Elasticsearch 6.x:: Elasticsearch 7.x:: -* The `type` parameter in URLs are optional. For instance, indexing +* The `type` parameter in URLs are deprecated. For instance, indexing a document no longer requires a document `type`. The new index APIs are `PUT {index}/_doc/{id}` in case of explicit ids and `POST {index}/_doc` for auto-generated ids. -* The `GET|PUT _mapping` APIs support a query string parameter - (`include_type_name`) which indicates whether the body should include - a layer for the type name. It defaults to `true`. 7.x indices which - don't have an explicit type will use the dummy type name `_doc`. +* The index creation, `GET|PUT _mapping` and document APIs support a query + string parameter (`include_type_name`) which indicates whether requests and + responses should include a type name. It defaults to `true`. + 7.x indices which don't have an explicit type will use the dummy type name + `_doc`. Not setting `include_type_name=false` will result in a deprecation + warning. * The `_default_` mapping type is removed. @@ -274,7 +276,8 @@ Elasticsearch 8.x:: * The `type` parameter is no longer supported in URLs. -* The `include_type_name` parameter defaults to `false`. +* The `include_type_name` parameter is deprecated, default to `false` and fails + the request when set to `true`. Elasticsearch 9.x:: From 24763d881e027ae30341a7bd0959513081d3ea15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 19 Apr 2018 16:48:17 +0200 Subject: [PATCH 04/11] Deprecate use of `htmlStrip` as name for HtmlStripCharFilter (#27429) The camel case name `htmlStip` should be removed in favour of `html_strip`, but we need to deprecate it first. This change adds deprecation warnings for indices with version starting with 6.3.0 and logs deprecation warnings in this cases. --- .../analysis/common/CommonAnalysisPlugin.java | 15 +++- .../HtmlStripCharFilterFactoryTests.java | 73 +++++++++++++++++++ .../test/indices.analyze/10_analyze.yml | 53 ++++++++++++++ .../analysis/PreConfiguredCharFilter.java | 9 +++ 4 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java diff --git a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java index e0193e50313f3..a01eb52fdd498 100644 --- a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java +++ b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java @@ -67,6 +67,8 @@ import org.apache.lucene.analysis.standard.ClassicFilter; import org.apache.lucene.analysis.tr.ApostropheFilter; import org.apache.lucene.analysis.util.ElisionFilter; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.index.analysis.CharFilterFactory; import org.elasticsearch.index.analysis.PreConfiguredCharFilter; import org.elasticsearch.index.analysis.PreConfiguredTokenFilter; @@ -88,6 +90,9 @@ import static org.elasticsearch.plugins.AnalysisPlugin.requriesAnalysisSettings; public class CommonAnalysisPlugin extends Plugin implements AnalysisPlugin { + + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(CommonAnalysisPlugin.class)); + @Override public Map> getTokenFilters() { Map> filters = new TreeMap<>(); @@ -171,8 +176,14 @@ public Map> getTokenizers() { public List getPreConfiguredCharFilters() { List filters = new ArrayList<>(); filters.add(PreConfiguredCharFilter.singleton("html_strip", false, HTMLStripCharFilter::new)); - // TODO deprecate htmlStrip - filters.add(PreConfiguredCharFilter.singleton("htmlStrip", false, HTMLStripCharFilter::new)); + filters.add(PreConfiguredCharFilter.singletonWithVersion("htmlStrip", false, (reader, version) -> { + if (version.onOrAfter(org.elasticsearch.Version.V_6_3_0)) { + DEPRECATION_LOGGER.deprecatedAndMaybeLog("htmlStrip_deprecation", + "The [htmpStrip] char filter name is deprecated and will be removed in a future version. " + + "Please change the filter name to [html_strip] instead."); + } + return new HTMLStripCharFilter(reader); + })); return filters; } diff --git a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java new file mode 100644 index 0000000000000..0d5389a6d6594 --- /dev/null +++ b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java @@ -0,0 +1,73 @@ +/* + * 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.analysis.common; + +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; +import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.analysis.CharFilterFactory; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.IndexSettingsModule; +import org.elasticsearch.test.VersionUtils; + +import java.io.IOException; +import java.io.StringReader; +import java.util.Map; + + +public class HtmlStripCharFilterFactoryTests extends ESTestCase { + + /** + * Check that the deprecated name "htmlStrip" issues a deprecation warning for indices created since 6.3.0 + */ + public void testDeprecationWarning() throws IOException { + Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) + .put(IndexMetaData.SETTING_VERSION_CREATED, VersionUtils.randomVersionBetween(random(), Version.V_6_3_0, Version.CURRENT)) + .build(); + + IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings); + try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) { + Map charFilters = createTestAnalysis(idxSettings, settings, commonAnalysisPlugin).charFilter; + CharFilterFactory charFilterFactory = charFilters.get("htmlStrip"); + assertNotNull(charFilterFactory.create(new StringReader("input"))); + assertWarnings("The [htmpStrip] char filter name is deprecated and will be removed in a future version. " + + "Please change the filter name to [html_strip] instead."); + } + } + + /** + * Check that the deprecated name "htmlStrip" does NOT issues a deprecation warning for indices created before 6.3.0 + */ + public void testNoDeprecationWarningPre6_3() throws IOException { + Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) + .put(IndexMetaData.SETTING_VERSION_CREATED, + VersionUtils.randomVersionBetween(random(), Version.V_5_0_0, Version.V_6_2_4)) + .build(); + + IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings); + try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) { + Map charFilters = createTestAnalysis(idxSettings, settings, commonAnalysisPlugin).charFilter; + CharFilterFactory charFilterFactory = charFilters.get("htmlStrip"); + assertNotNull(charFilterFactory.create(new StringReader(""))); + } + } +} diff --git a/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml b/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml index cbb8f053cfbba..f8fc3acc02c4c 100644 --- a/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml +++ b/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml @@ -17,3 +17,56 @@ - match: { error.type: illegal_argument_exception } - match: { error.reason: "Custom normalizer may not use filter [word_delimiter]" } +--- +"htmlStrip_deprecated": + - skip: + version: " - 6.2.99" + reason: deprecated in 6.3 + features: "warnings" + + - do: + indices.create: + index: test_deprecated_htmlstrip + body: + settings: + index: + analysis: + analyzer: + my_htmlStripWithCharfilter: + tokenizer: keyword + char_filter: ["htmlStrip"] + mappings: + type: + properties: + name: + type: text + analyzer: my_htmlStripWithCharfilter + + - do: + warnings: + - 'The [htmpStrip] char filter name is deprecated and will be removed in a future version. Please change the filter name to [html_strip] instead.' + index: + index: test_deprecated_htmlstrip + type: type + id: 1 + body: { "name": "foo bar" } + + - do: + warnings: + - 'The [htmpStrip] char filter name is deprecated and will be removed in a future version. Please change the filter name to [html_strip] instead.' + index: + index: test_deprecated_htmlstrip + type: type + id: 2 + body: { "name": "foo baz" } + + - do: + warnings: + - 'The [htmpStrip] char filter name is deprecated and will be removed in a future version. Please change the filter name to [html_strip] instead.' + indices.analyze: + index: test_deprecated_htmlstrip + body: + analyzer: "my_htmlStripWithCharfilter" + text: "foo" + - length: { tokens: 1 } + - match: { tokens.0.token: "\nfoo\n" } diff --git a/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java b/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java index a979e9e34fe4e..84eb0c4c3498c 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java @@ -40,6 +40,15 @@ public static PreConfiguredCharFilter singleton(String name, boolean useFilterFo (reader, version) -> create.apply(reader)); } + /** + * Create a pre-configured char filter that may not vary at all, provide access to the elasticsearch verison + */ + public static PreConfiguredCharFilter singletonWithVersion(String name, boolean useFilterForMultitermQueries, + BiFunction create) { + return new PreConfiguredCharFilter(name, CachingStrategy.ONE, useFilterForMultitermQueries, + (reader, version) -> create.apply(reader, version)); + } + /** * Create a pre-configured token filter that may vary based on the Lucene version. */ From d0f6657d90579e3b1e436a173f5d98e797caf26f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 19 Apr 2018 17:00:52 +0200 Subject: [PATCH 05/11] Add tests for ranking evaluation with aliases (#29452) The ranking evaluation requests so far were not tested against aliases but they should run regardless of the targeted index is a real index or an alias. This change adds cases for this to the integration and rest tests. --- .../index/rankeval/RankEvalRequestIT.java | 50 ++++++----- .../rest-api-spec/test/rank_eval/10_basic.yml | 90 +++++++++---------- 2 files changed, 73 insertions(+), 67 deletions(-) diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java index 8a3bad50b22da..b55c57bae2bcf 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.index.IndexNotFoundException; @@ -40,10 +41,13 @@ import java.util.Set; import static org.elasticsearch.index.rankeval.EvaluationMetric.filterUnknownDocuments; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.instanceOf; public class RankEvalRequestIT extends ESIntegTestCase { + private static final String TEST_INDEX = "test"; + private static final String INDEX_ALIAS = "alias0"; private static final int RELEVANT_RATING_1 = 1; @Override @@ -58,20 +62,23 @@ protected Collection> nodePlugins() { @Before public void setup() { - createIndex("test"); + createIndex(TEST_INDEX); ensureGreen(); - client().prepareIndex("test", "testtype").setId("1") + client().prepareIndex(TEST_INDEX, "testtype").setId("1") .setSource("text", "berlin", "title", "Berlin, Germany", "population", 3670622).get(); - client().prepareIndex("test", "testtype").setId("2").setSource("text", "amsterdam", "population", 851573).get(); - client().prepareIndex("test", "testtype").setId("3").setSource("text", "amsterdam", "population", 851573).get(); - client().prepareIndex("test", "testtype").setId("4").setSource("text", "amsterdam", "population", 851573).get(); - client().prepareIndex("test", "testtype").setId("5").setSource("text", "amsterdam", "population", 851573).get(); - client().prepareIndex("test", "testtype").setId("6").setSource("text", "amsterdam", "population", 851573).get(); + client().prepareIndex(TEST_INDEX, "testtype").setId("2").setSource("text", "amsterdam", "population", 851573).get(); + client().prepareIndex(TEST_INDEX, "testtype").setId("3").setSource("text", "amsterdam", "population", 851573).get(); + client().prepareIndex(TEST_INDEX, "testtype").setId("4").setSource("text", "amsterdam", "population", 851573).get(); + client().prepareIndex(TEST_INDEX, "testtype").setId("5").setSource("text", "amsterdam", "population", 851573).get(); + client().prepareIndex(TEST_INDEX, "testtype").setId("6").setSource("text", "amsterdam", "population", 851573).get(); // add another index for testing closed indices etc... client().prepareIndex("test2", "testtype").setId("7").setSource("text", "amsterdam", "population", 851573).get(); refresh(); + + // set up an alias that can also be used in tests + assertAcked(client().admin().indices().prepareAliases().addAliasAction(AliasActions.add().index(TEST_INDEX).alias(INDEX_ALIAS))); } /** @@ -101,7 +108,8 @@ public void testPrecisionAtRequest() { RankEvalAction.INSTANCE, new RankEvalRequest()); builder.setRankEvalSpec(task); - RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, builder.request().indices("test")) + String indexToUse = randomBoolean() ? TEST_INDEX : INDEX_ALIAS; + RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, builder.request().indices(indexToUse)) .actionGet(); // the expected Prec@ for the first query is 4/6 and the expected Prec@ for the // second is 1/6, divided by 2 to get the average @@ -143,7 +151,7 @@ public void testPrecisionAtRequest() { metric = new PrecisionAtK(1, false, 3); task = new RankEvalSpec(specifications, metric); - builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, new RankEvalRequest(task, new String[] { "test" })); + builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, new RankEvalRequest(task, new String[] { TEST_INDEX })); response = client().execute(RankEvalAction.INSTANCE, builder.request()).actionGet(); // if we look only at top 3 documente, the expected P@3 for the first query is @@ -163,19 +171,19 @@ public void testDCGRequest() { List specifications = new ArrayList<>(); List ratedDocs = Arrays.asList( - new RatedDocument("test", "1", 3), - new RatedDocument("test", "2", 2), - new RatedDocument("test", "3", 3), - new RatedDocument("test", "4", 0), - new RatedDocument("test", "5", 1), - new RatedDocument("test", "6", 2)); + new RatedDocument(TEST_INDEX, "1", 3), + new RatedDocument(TEST_INDEX, "2", 2), + new RatedDocument(TEST_INDEX, "3", 3), + new RatedDocument(TEST_INDEX, "4", 0), + new RatedDocument(TEST_INDEX, "5", 1), + new RatedDocument(TEST_INDEX, "6", 2)); specifications.add(new RatedRequest("amsterdam_query", ratedDocs, testQuery)); DiscountedCumulativeGain metric = new DiscountedCumulativeGain(false, null, 10); RankEvalSpec task = new RankEvalSpec(specifications, metric); RankEvalRequestBuilder builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, - new RankEvalRequest(task, new String[] { "test" })); + new RankEvalRequest(task, new String[] { TEST_INDEX })); RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, builder.request()).actionGet(); assertEquals(DiscountedCumulativeGainTests.EXPECTED_DCG, response.getEvaluationResult(), 10E-14); @@ -184,7 +192,7 @@ public void testDCGRequest() { metric = new DiscountedCumulativeGain(false, null, 3); task = new RankEvalSpec(specifications, metric); - builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, new RankEvalRequest(task, new String[] { "test" })); + builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, new RankEvalRequest(task, new String[] { TEST_INDEX })); response = client().execute(RankEvalAction.INSTANCE, builder.request()).actionGet(); assertEquals(12.39278926071437, response.getEvaluationResult(), 10E-14); @@ -203,7 +211,7 @@ public void testMRRRequest() { RankEvalSpec task = new RankEvalSpec(specifications, metric); RankEvalRequestBuilder builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, - new RankEvalRequest(task, new String[] { "test" })); + new RankEvalRequest(task, new String[] { TEST_INDEX })); RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, builder.request()).actionGet(); // the expected reciprocal rank for the amsterdam_query is 1/5 @@ -216,7 +224,7 @@ public void testMRRRequest() { metric = new MeanReciprocalRank(1, 3); task = new RankEvalSpec(specifications, metric); - builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, new RankEvalRequest(task, new String[] { "test" })); + builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, new RankEvalRequest(task, new String[] { TEST_INDEX })); response = client().execute(RankEvalAction.INSTANCE, builder.request()).actionGet(); // limiting to top 3 results, the amsterdam_query has no relevant document in it @@ -247,7 +255,7 @@ public void testBadQuery() { RankEvalSpec task = new RankEvalSpec(specifications, new PrecisionAtK()); RankEvalRequestBuilder builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, - new RankEvalRequest(task, new String[] { "test" })); + new RankEvalRequest(task, new String[] { TEST_INDEX })); builder.setRankEvalSpec(task); RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, builder.request()).actionGet(); @@ -267,7 +275,7 @@ public void testIndicesOptions() { specifications.add(new RatedRequest("amsterdam_query", relevantDocs, amsterdamQuery)); RankEvalSpec task = new RankEvalSpec(specifications, new PrecisionAtK()); - RankEvalRequest request = new RankEvalRequest(task, new String[] { "test", "test2" }); + RankEvalRequest request = new RankEvalRequest(task, new String[] { TEST_INDEX, "test2" }); request.setRankEvalSpec(task); RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, request).actionGet(); diff --git a/modules/rank-eval/src/test/resources/rest-api-spec/test/rank_eval/10_basic.yml b/modules/rank-eval/src/test/resources/rest-api-spec/test/rank_eval/10_basic.yml index fcf5f945a06ae..3900b1f32baa7 100644 --- a/modules/rank-eval/src/test/resources/rest-api-spec/test/rank_eval/10_basic.yml +++ b/modules/rank-eval/src/test/resources/rest-api-spec/test/rank_eval/10_basic.yml @@ -1,10 +1,4 @@ ---- -"Response format": - - - skip: - version: " - 6.2.99" - reason: response format was updated in 6.3 - +setup: - do: indices.create: index: foo @@ -43,8 +37,21 @@ - do: indices.refresh: {} + - do: + indices.put_alias: + index: foo + name: alias + +--- +"Response format": + + - skip: + version: " - 6.2.99" + reason: response format was updated in 6.3 + - do: rank_eval: + index: foo, body: { "requests" : [ { @@ -84,52 +91,43 @@ - match: { details.berlin_query.hits.0.hit._id: "doc1" } - match: { details.berlin_query.hits.0.rating: 1} - match: { details.berlin_query.hits.1.hit._id: "doc4" } - - is_false: details.berlin_query.hits.1.rating + - is_false: details.berlin_query.hits.1.rating --- -"Mean Reciprocal Rank": - - - skip: - version: " - 6.2.99" - reason: response format was updated in 6.3 +"Alias resolution": - do: - indices.create: - index: foo - body: - settings: - index: - number_of_shards: 1 - - do: - index: - index: foo - type: bar - id: doc1 - body: { "text": "berlin" } + rank_eval: + index: alias + body: { + "requests" : [ + { + "id": "amsterdam_query", + "request": { "query": { "match" : {"text" : "amsterdam" }}}, + "ratings": [ + {"_index": "foo", "_id": "doc1", "rating": 0}, + {"_index": "foo", "_id": "doc2", "rating": 1}, + {"_index": "foo", "_id": "doc3", "rating": 1}] + }, + { + "id" : "berlin_query", + "request": { "query": { "match" : { "text" : "berlin" } }, "size" : 10 }, + "ratings": [{"_index": "foo", "_id": "doc1", "rating": 1}] + } + ], + "metric" : { "precision": { "ignore_unlabeled" : true }} + } - - do: - index: - index: foo - type: bar - id: doc2 - body: { "text": "amsterdam" } + - match: { quality_level: 1} + - match: { details.amsterdam_query.quality_level: 1.0} + - match: { details.berlin_query.quality_level: 1.0} - - do: - index: - index: foo - type: bar - id: doc3 - body: { "text": "amsterdam" } - - - do: - index: - index: foo - type: bar - id: doc4 - body: { "text": "something about amsterdam and berlin" } +--- +"Mean Reciprocal Rank": - - do: - indices.refresh: {} + - skip: + version: " - 6.2.99" + reason: response format was updated in 6.3 - do: rank_eval: From 7fa7dea044dc162f8a787116fa6bfbb54b6025fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 19 Apr 2018 18:11:06 +0200 Subject: [PATCH 06/11] [Tests] Remove accidental logger usage --- .../client/documentation/SearchDocumentationIT.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java index a8dde67fa94a0..52f6984e65107 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java @@ -37,7 +37,6 @@ import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.client.ESRestHighLevelClientTestCase; import org.elasticsearch.client.RestHighLevelClient; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.text.Text; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.unit.TimeValue; @@ -723,7 +722,6 @@ public void testRankEval() throws Exception { RankEvalResponse response = client.rankEval(request); // end::rank-eval-execute - logger.warn(Strings.toString(response)); // tag::rank-eval-response double evaluationResult = response.getEvaluationResult(); // <1> assertEquals(1.0 / 3.0, evaluationResult, 0.0); From 1b24d4e68b626afb1b1f5499434be84765d16015 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 19 Apr 2018 12:38:10 -0400 Subject: [PATCH 07/11] Avoid side-effect in VersionMap when assertion enabled (#29585) Today when a version map does not require safe access, we will skip that document. However, if the assertion is enabled, we remove the delete tombstone of that document if existed. This side-effect may accidentally hide bugs in which stale delete tombstone can be accessed. This change ensures putAssertionMap not modify the tombstone maps. --- .../index/engine/DeleteVersionValue.java | 2 +- ...rsionValue.java => IndexVersionValue.java} | 10 +-- .../index/engine/InternalEngine.java | 16 ++--- .../index/engine/LiveVersionMap.java | 51 +++++++-------- .../index/engine/VersionValue.java | 2 +- .../index/engine/LiveVersionMapTests.java | 65 ++++++++++++------- .../index/engine/VersionValueTests.java | 9 ++- 7 files changed, 83 insertions(+), 72 deletions(-) rename server/src/main/java/org/elasticsearch/index/engine/{TranslogVersionValue.java => IndexVersionValue.java} (86%) diff --git a/server/src/main/java/org/elasticsearch/index/engine/DeleteVersionValue.java b/server/src/main/java/org/elasticsearch/index/engine/DeleteVersionValue.java index d2b2e24c616a8..9f094197b8d9c 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/DeleteVersionValue.java +++ b/server/src/main/java/org/elasticsearch/index/engine/DeleteVersionValue.java @@ -23,7 +23,7 @@ /** Holds a deleted version, which just adds a timestamp to {@link VersionValue} so we know when we can expire the deletion. */ -class DeleteVersionValue extends VersionValue { +final class DeleteVersionValue extends VersionValue { private static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(DeleteVersionValue.class); diff --git a/server/src/main/java/org/elasticsearch/index/engine/TranslogVersionValue.java b/server/src/main/java/org/elasticsearch/index/engine/IndexVersionValue.java similarity index 86% rename from server/src/main/java/org/elasticsearch/index/engine/TranslogVersionValue.java rename to server/src/main/java/org/elasticsearch/index/engine/IndexVersionValue.java index 67415ea6139a6..4f67372926712 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/TranslogVersionValue.java +++ b/server/src/main/java/org/elasticsearch/index/engine/IndexVersionValue.java @@ -24,13 +24,13 @@ import java.util.Objects; -final class TranslogVersionValue extends VersionValue { +final class IndexVersionValue extends VersionValue { - private static final long RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(TranslogVersionValue.class); + private static final long RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(IndexVersionValue.class); private final Translog.Location translogLocation; - TranslogVersionValue(Translog.Location translogLocation, long version, long seqNo, long term) { + IndexVersionValue(Translog.Location translogLocation, long version, long seqNo, long term) { super(version, seqNo, term); this.translogLocation = translogLocation; } @@ -45,7 +45,7 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; if (!super.equals(o)) return false; - TranslogVersionValue that = (TranslogVersionValue) o; + IndexVersionValue that = (IndexVersionValue) o; return Objects.equals(translogLocation, that.translogLocation); } @@ -56,7 +56,7 @@ public int hashCode() { @Override public String toString() { - return "TranslogVersionValue{" + + return "IndexVersionValue{" + "version=" + version + ", seqNo=" + seqNo + ", term=" + term + diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index b28a5cd59e25b..ab13639ce2fd2 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -623,7 +623,7 @@ private VersionValue resolveDocVersion(final Operation op) throws IOException { assert incrementIndexVersionLookup(); // used for asserting in tests final long currentVersion = loadCurrentVersionFromIndex(op.uid()); if (currentVersion != Versions.NOT_FOUND) { - versionValue = new VersionValue(currentVersion, SequenceNumbers.UNASSIGNED_SEQ_NO, 0L); + versionValue = new IndexVersionValue(null, currentVersion, SequenceNumbers.UNASSIGNED_SEQ_NO, 0L); } } else if (engineConfig.isEnableGcDeletes() && versionValue.isDelete() && (engineConfig.getThreadPool().relativeTimeInMillis() - ((DeleteVersionValue)versionValue).time) > getGcDeletesInMillis()) { @@ -785,8 +785,9 @@ public IndexResult index(Index index) throws IOException { indexResult.setTranslogLocation(location); } if (plan.indexIntoLucene && indexResult.hasFailure() == false) { - versionMap.maybePutUnderLock(index.uid().bytes(), - getVersionValue(plan.versionForIndexing, plan.seqNoForIndexing, index.primaryTerm(), indexResult.getTranslogLocation())); + final Translog.Location translogLocation = trackTranslogLocation.get() ? indexResult.getTranslogLocation() : null; + versionMap.maybePutIndexUnderLock(index.uid().bytes(), + new IndexVersionValue(translogLocation, plan.versionForIndexing, plan.seqNoForIndexing, index.primaryTerm())); } if (indexResult.getSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO) { localCheckpointTracker.markSeqNoAsCompleted(indexResult.getSeqNo()); @@ -937,13 +938,6 @@ private IndexResult indexIntoLucene(Index index, IndexingStrategy plan) } } - private VersionValue getVersionValue(long version, long seqNo, long term, Translog.Location location) { - if (location != null && trackTranslogLocation.get()) { - return new TranslogVersionValue(location, version, seqNo, term); - } - return new VersionValue(version, seqNo, term); - } - /** * returns true if the indexing operation may have already be processed by this engine. * Note that it is OK to rarely return true even if this is not the case. However a `false` @@ -1193,7 +1187,7 @@ private DeleteResult deleteInLucene(Delete delete, DeletionStrategy plan) indexWriter.deleteDocuments(delete.uid()); numDocDeletes.inc(); } - versionMap.putUnderLock(delete.uid().bytes(), + versionMap.putDeleteUnderLock(delete.uid().bytes(), new DeleteVersionValue(plan.versionOfDeletion, plan.seqNoOfDeletion, delete.primaryTerm(), engineConfig.getThreadPool().relativeTimeInMillis())); return new DeleteResult( diff --git a/server/src/main/java/org/elasticsearch/index/engine/LiveVersionMap.java b/server/src/main/java/org/elasticsearch/index/engine/LiveVersionMap.java index 7c5dcfa5c9050..6d9dc4a38974c 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/LiveVersionMap.java +++ b/server/src/main/java/org/elasticsearch/index/engine/LiveVersionMap.java @@ -268,7 +268,7 @@ VersionValue getUnderLock(final BytesRef uid) { } private VersionValue getUnderLock(final BytesRef uid, Maps currentMaps) { - assert keyedLock.isHeldByCurrentThread(uid); + assert assertKeyedLockHeldByCurrentThread(uid); // First try to get the "live" value: VersionValue value = currentMaps.current.get(uid); if (value != null) { @@ -306,44 +306,36 @@ boolean isSafeAccessRequired() { /** * Adds this uid/version to the pending adds map iff the map needs safe access. */ - void maybePutUnderLock(BytesRef uid, VersionValue version) { - assert keyedLock.isHeldByCurrentThread(uid); + void maybePutIndexUnderLock(BytesRef uid, IndexVersionValue version) { + assert assertKeyedLockHeldByCurrentThread(uid); Maps maps = this.maps; if (maps.isSafeAccessMode()) { - putUnderLock(uid, version, maps); + putIndexUnderLock(uid, version); } else { maps.current.markAsUnsafe(); assert putAssertionMap(uid, version); } } - private boolean putAssertionMap(BytesRef uid, VersionValue version) { - putUnderLock(uid, version, unsafeKeysMap); - return true; + void putIndexUnderLock(BytesRef uid, IndexVersionValue version) { + assert assertKeyedLockHeldByCurrentThread(uid); + assert uid.bytes.length == uid.length : "Oversized _uid! UID length: " + uid.length + ", bytes length: " + uid.bytes.length; + maps.put(uid, version); + removeTombstoneUnderLock(uid); } - /** - * Adds this uid/version to the pending adds map. - */ - void putUnderLock(BytesRef uid, VersionValue version) { - Maps maps = this.maps; - putUnderLock(uid, version, maps); + private boolean putAssertionMap(BytesRef uid, IndexVersionValue version) { + assert assertKeyedLockHeldByCurrentThread(uid); + assert uid.bytes.length == uid.length : "Oversized _uid! UID length: " + uid.length + ", bytes length: " + uid.bytes.length; + unsafeKeysMap.put(uid, version); + return true; } - /** - * Adds this uid/version to the pending adds map. - */ - private void putUnderLock(BytesRef uid, VersionValue version, Maps maps) { - assert keyedLock.isHeldByCurrentThread(uid); + void putDeleteUnderLock(BytesRef uid, DeleteVersionValue version) { + assert assertKeyedLockHeldByCurrentThread(uid); assert uid.bytes.length == uid.length : "Oversized _uid! UID length: " + uid.length + ", bytes length: " + uid.bytes.length; - if (version.isDelete() == false) { - maps.put(uid, version); - removeTombstoneUnderLock(uid); - } else { - DeleteVersionValue versionValue = (DeleteVersionValue) version; - putTombstone(uid, versionValue); - maps.remove(uid, versionValue); - } + putTombstone(uid, version); + maps.remove(uid, version); } private void putTombstone(BytesRef uid, DeleteVersionValue version) { @@ -365,7 +357,7 @@ private void putTombstone(BytesRef uid, DeleteVersionValue version) { * Removes this uid from the pending deletes map. */ void removeTombstoneUnderLock(BytesRef uid) { - assert keyedLock.isHeldByCurrentThread(uid); + assert assertKeyedLockHeldByCurrentThread(uid); long uidRAMBytesUsed = BASE_BYTES_PER_BYTESREF + uid.bytes.length; final VersionValue prev = tombstones.remove(uid); if (prev != null) { @@ -465,4 +457,9 @@ Map getAllTombstones() { Releasable acquireLock(BytesRef uid) { return keyedLock.acquire(uid); } + + private boolean assertKeyedLockHeldByCurrentThread(BytesRef uid) { + assert keyedLock.isHeldByCurrentThread(uid) : "Thread [" + Thread.currentThread().getName() + "], uid [" + uid.utf8ToString() + "]"; + return true; + } } diff --git a/server/src/main/java/org/elasticsearch/index/engine/VersionValue.java b/server/src/main/java/org/elasticsearch/index/engine/VersionValue.java index d63306486732e..567a7964186ad 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/VersionValue.java +++ b/server/src/main/java/org/elasticsearch/index/engine/VersionValue.java @@ -27,7 +27,7 @@ import java.util.Collection; import java.util.Collections; -class VersionValue implements Accountable { +abstract class VersionValue implements Accountable { private static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(VersionValue.class); diff --git a/server/src/test/java/org/elasticsearch/index/engine/LiveVersionMapTests.java b/server/src/test/java/org/elasticsearch/index/engine/LiveVersionMapTests.java index ce3ddff00dade..e0efcf9f0f73f 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/LiveVersionMapTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/LiveVersionMapTests.java @@ -25,6 +25,7 @@ import org.apache.lucene.util.RamUsageTester; import org.apache.lucene.util.TestUtil; import org.elasticsearch.common.lease.Releasable; +import org.elasticsearch.index.translog.Translog; import org.elasticsearch.test.ESTestCase; import java.io.IOException; @@ -47,9 +48,8 @@ public void testRamBytesUsed() throws Exception { for (int i = 0; i < 100000; ++i) { BytesRefBuilder uid = new BytesRefBuilder(); uid.copyChars(TestUtil.randomSimpleString(random(), 10, 20)); - VersionValue version = new VersionValue(randomLong(), randomLong(), randomLong()); try (Releasable r = map.acquireLock(uid.toBytesRef())) { - map.putUnderLock(uid.toBytesRef(), version); + map.putIndexUnderLock(uid.toBytesRef(), randomIndexVersionValue()); } } long actualRamBytesUsed = RamUsageTester.sizeOf(map); @@ -64,9 +64,8 @@ public void testRamBytesUsed() throws Exception { for (int i = 0; i < 100000; ++i) { BytesRefBuilder uid = new BytesRefBuilder(); uid.copyChars(TestUtil.randomSimpleString(random(), 10, 20)); - VersionValue version = new VersionValue(randomLong(), randomLong(), randomLong()); try (Releasable r = map.acquireLock(uid.toBytesRef())) { - map.putUnderLock(uid.toBytesRef(), version); + map.putIndexUnderLock(uid.toBytesRef(), randomIndexVersionValue()); } } actualRamBytesUsed = RamUsageTester.sizeOf(map); @@ -100,14 +99,15 @@ private BytesRef uid(String string) { public void testBasics() throws IOException { LiveVersionMap map = new LiveVersionMap(); try (Releasable r = map.acquireLock(uid("test"))) { - map.putUnderLock(uid("test"), new VersionValue(1,1,1)); - assertEquals(new VersionValue(1,1,1), map.getUnderLock(uid("test"))); + Translog.Location tlogLoc = randomTranslogLocation(); + map.putIndexUnderLock(uid("test"), new IndexVersionValue(tlogLoc, 1, 1, 1)); + assertEquals(new IndexVersionValue(tlogLoc, 1, 1, 1), map.getUnderLock(uid("test"))); map.beforeRefresh(); - assertEquals(new VersionValue(1,1,1), map.getUnderLock(uid("test"))); + assertEquals(new IndexVersionValue(tlogLoc, 1, 1, 1), map.getUnderLock(uid("test"))); map.afterRefresh(randomBoolean()); assertNull(map.getUnderLock(uid("test"))); - map.putUnderLock(uid("test"), new DeleteVersionValue(1,1,1,1)); + map.putDeleteUnderLock(uid("test"), new DeleteVersionValue(1,1,1,1)); assertEquals(new DeleteVersionValue(1,1,1,1), map.getUnderLock(uid("test"))); map.beforeRefresh(); assertEquals(new DeleteVersionValue(1,1,1,1), map.getUnderLock(uid("test"))); @@ -154,21 +154,24 @@ public void testConcurrently() throws IOException, InterruptedException { BytesRef bytesRef = randomFrom(random(), keyList); try (Releasable r = map.acquireLock(bytesRef)) { VersionValue versionValue = values.computeIfAbsent(bytesRef, - v -> new VersionValue(randomLong(), maxSeqNo.incrementAndGet(), randomLong())); + v -> new IndexVersionValue( + randomTranslogLocation(), randomLong(), maxSeqNo.incrementAndGet(), randomLong())); boolean isDelete = versionValue instanceof DeleteVersionValue; if (isDelete) { map.removeTombstoneUnderLock(bytesRef); deletes.remove(bytesRef); } if (isDelete == false && rarely()) { - versionValue = new DeleteVersionValue(versionValue.version + 1, maxSeqNo.incrementAndGet(), - versionValue.term, clock.getAndIncrement()); + versionValue = new DeleteVersionValue(versionValue.version + 1, + maxSeqNo.incrementAndGet(), versionValue.term, clock.getAndIncrement()); deletes.put(bytesRef, (DeleteVersionValue) versionValue); + map.putDeleteUnderLock(bytesRef, (DeleteVersionValue) versionValue); } else { - versionValue = new VersionValue(versionValue.version + 1, maxSeqNo.incrementAndGet(), versionValue.term); + versionValue = new IndexVersionValue(randomTranslogLocation(), + versionValue.version + 1, maxSeqNo.incrementAndGet(), versionValue.term); + map.putIndexUnderLock(bytesRef, (IndexVersionValue) versionValue); } values.put(bytesRef, versionValue); - map.putUnderLock(bytesRef, versionValue); } if (rarely()) { final long pruneSeqNo = randomLongBetween(0, maxSeqNo.get()); @@ -268,7 +271,7 @@ public void testCarryOnSafeAccess() throws IOException { } try (Releasable r = map.acquireLock(uid(""))) { - map.maybePutUnderLock(new BytesRef(""), new VersionValue(randomLong(), randomLong(), randomLong())); + map.maybePutIndexUnderLock(new BytesRef(""), randomIndexVersionValue()); } assertFalse(map.isUnsafe()); assertEquals(1, map.getAllCurrent().size()); @@ -278,7 +281,7 @@ public void testCarryOnSafeAccess() throws IOException { assertFalse(map.isUnsafe()); assertFalse(map.isSafeAccessRequired()); try (Releasable r = map.acquireLock(uid(""))) { - map.maybePutUnderLock(new BytesRef(""), new VersionValue(randomLong(), randomLong(), randomLong())); + map.maybePutIndexUnderLock(new BytesRef(""), randomIndexVersionValue()); } assertTrue(map.isUnsafe()); assertFalse(map.isSafeAccessRequired()); @@ -288,7 +291,7 @@ public void testCarryOnSafeAccess() throws IOException { public void testRefreshTransition() throws IOException { LiveVersionMap map = new LiveVersionMap(); try (Releasable r = map.acquireLock(uid("1"))) { - map.maybePutUnderLock(uid("1"), new VersionValue(randomLong(), randomLong(), randomLong())); + map.maybePutIndexUnderLock(uid("1"), randomIndexVersionValue()); assertTrue(map.isUnsafe()); assertNull(map.getUnderLock(uid("1"))); map.beforeRefresh(); @@ -299,7 +302,7 @@ public void testRefreshTransition() throws IOException { assertFalse(map.isUnsafe()); map.enforceSafeAccess(); - map.maybePutUnderLock(uid("1"), new VersionValue(randomLong(), randomLong(), randomLong())); + map.maybePutIndexUnderLock(uid("1"), randomIndexVersionValue()); assertFalse(map.isUnsafe()); assertNotNull(map.getUnderLock(uid("1"))); map.beforeRefresh(); @@ -320,9 +323,10 @@ public void testAddAndDeleteRefreshConcurrently() throws IOException, Interrupte AtomicLong version = new AtomicLong(); CountDownLatch start = new CountDownLatch(2); BytesRef uid = uid("1"); - VersionValue initialVersion = new VersionValue(version.incrementAndGet(), 1, 1); + VersionValue initialVersion; try (Releasable ignore = map.acquireLock(uid)) { - map.putUnderLock(uid, initialVersion); + initialVersion = new IndexVersionValue(randomTranslogLocation(), version.incrementAndGet(), 1, 1); + map.putIndexUnderLock(uid, (IndexVersionValue) initialVersion); } Thread t = new Thread(() -> { start.countDown(); @@ -337,14 +341,13 @@ public void testAddAndDeleteRefreshConcurrently() throws IOException, Interrupte } else { underLock = nextVersionValue; } - if (underLock.isDelete()) { - nextVersionValue = new VersionValue(version.incrementAndGet(), 1, 1); - } else if (randomBoolean()) { - nextVersionValue = new VersionValue(version.incrementAndGet(), 1, 1); + if (underLock.isDelete() || randomBoolean()) { + nextVersionValue = new IndexVersionValue(randomTranslogLocation(), version.incrementAndGet(), 1, 1); + map.putIndexUnderLock(uid, (IndexVersionValue) nextVersionValue); } else { nextVersionValue = new DeleteVersionValue(version.incrementAndGet(), 1, 1, 0); + map.putDeleteUnderLock(uid, (DeleteVersionValue) nextVersionValue); } - map.putUnderLock(uid, nextVersionValue); } } } catch (Exception e) { @@ -375,7 +378,7 @@ public void testPruneTombstonesWhileLocked() throws InterruptedException, IOExce BytesRef uid = uid("1"); ; try (Releasable ignore = map.acquireLock(uid)) { - map.putUnderLock(uid, new DeleteVersionValue(0, 0, 0, 0)); + map.putDeleteUnderLock(uid, new DeleteVersionValue(0, 0, 0, 0)); map.beforeRefresh(); // refresh otherwise we won't prune since it's tracked by the current map map.afterRefresh(false); Thread thread = new Thread(() -> { @@ -392,4 +395,16 @@ public void testPruneTombstonesWhileLocked() throws InterruptedException, IOExce thread.join(); assertEquals(0, map.getAllTombstones().size()); } + + IndexVersionValue randomIndexVersionValue() { + return new IndexVersionValue(randomTranslogLocation(), randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()); + } + + Translog.Location randomTranslogLocation() { + if (randomBoolean()) { + return null; + } else { + return new Translog.Location(randomNonNegativeLong(), randomNonNegativeLong(), randomInt()); + } + } } diff --git a/server/src/test/java/org/elasticsearch/index/engine/VersionValueTests.java b/server/src/test/java/org/elasticsearch/index/engine/VersionValueTests.java index 3b953edece1b4..242a568295dd6 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/VersionValueTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/VersionValueTests.java @@ -20,12 +20,17 @@ package org.elasticsearch.index.engine; import org.apache.lucene.util.RamUsageTester; +import org.elasticsearch.index.translog.Translog; import org.elasticsearch.test.ESTestCase; public class VersionValueTests extends ESTestCase { - public void testRamBytesUsed() { - VersionValue versionValue = new VersionValue(randomLong(), randomLong(), randomLong()); + public void testIndexRamBytesUsed() { + Translog.Location translogLoc = null; + if (randomBoolean()) { + translogLoc = new Translog.Location(randomNonNegativeLong(), randomNonNegativeLong(), randomInt()); + } + IndexVersionValue versionValue = new IndexVersionValue(translogLoc, randomLong(), randomLong(), randomLong()); assertEquals(RamUsageTester.sizeOf(versionValue), versionValue.ramBytesUsed()); } From 4f282e9e32fb66885368d80ad50aca5aa3f93099 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 19 Apr 2018 09:51:52 -0700 Subject: [PATCH 08/11] Build: Move java home checks to pre-execution phase (#29548) This commit moves the checks on JAVAX_HOME (where X is the java version number) existing to the end of gradle's configuration phase, and based on whether the tasks needing the java home are configured to execute. relates #29519 --- .../elasticsearch/gradle/BuildPlugin.groovy | 42 ++++++++++++------- .../gradle/test/ClusterFormationTasks.groovy | 4 ++ .../elasticsearch/gradle/test/NodeInfo.groovy | 22 +++++++--- distribution/bwc/build.gradle | 4 +- qa/reindex-from-old/build.gradle | 2 +- 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index 3103f23472ed7..d0ae4fd470312 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -38,6 +38,7 @@ import org.gradle.api.artifacts.ModuleVersionIdentifier import org.gradle.api.artifacts.ProjectDependency import org.gradle.api.artifacts.ResolvedArtifact import org.gradle.api.artifacts.dsl.RepositoryHandler +import org.gradle.api.execution.TaskExecutionGraph import org.gradle.api.plugins.JavaPlugin import org.gradle.api.publish.maven.MavenPublication import org.gradle.api.publish.maven.plugins.MavenPublishPlugin @@ -221,21 +222,34 @@ class BuildPlugin implements Plugin { return System.getenv('JAVA' + version + '_HOME') } - /** - * Get Java home for the project for the specified version. If the specified version is not configured, an exception with the specified - * message is thrown. - * - * @param project the project - * @param version the version of Java home to obtain - * @param message the exception message if Java home for the specified version is not configured - * @return Java home for the specified version - * @throws GradleException if Java home for the specified version is not configured - */ - static String getJavaHome(final Project project, final int version, final String message) { - if (project.javaVersions.get(version) == null) { - throw new GradleException(message) + /** Add a check before gradle execution phase which ensures java home for the given java version is set. */ + static void requireJavaHome(Task task, int version) { + Project rootProject = task.project.rootProject // use root project for global accounting + if (rootProject.hasProperty('requiredJavaVersions') == false) { + rootProject.rootProject.ext.requiredJavaVersions = [:].withDefault{key -> return []} + rootProject.gradle.taskGraph.whenReady { TaskExecutionGraph taskGraph -> + List messages = [] + for (entry in rootProject.requiredJavaVersions) { + if (rootProject.javaVersions.get(entry.key) != null) { + continue + } + List tasks = entry.value.findAll { taskGraph.hasTask(it) }.collect { " ${it.path}" } + if (tasks.isEmpty() == false) { + messages.add("JAVA${entry.key}_HOME required to run tasks:\n${tasks.join('\n')}") + } + } + if (messages.isEmpty() == false) { + throw new GradleException(messages.join('\n')) + } + } } - return project.javaVersions.get(version) + rootProject.requiredJavaVersions.get(version).add(task) + } + + /** A convenience method for getting java home for a version of java and requiring that version for the given task to execute */ + static String getJavaHome(final Task task, final int version) { + requireJavaHome(task, version) + return task.project.javaVersions.get(version) } private static String findRuntimeJavaHome(final String compilerJavaHome) { diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy index 5f9e4c49b34e9..b26320b400cc9 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy @@ -20,6 +20,7 @@ package org.elasticsearch.gradle.test import org.apache.tools.ant.DefaultLogger import org.apache.tools.ant.taskdefs.condition.Os +import org.elasticsearch.gradle.BuildPlugin import org.elasticsearch.gradle.LoggedExec import org.elasticsearch.gradle.Version import org.elasticsearch.gradle.VersionProperties @@ -607,6 +608,9 @@ class ClusterFormationTasks { } Task start = project.tasks.create(name: name, type: DefaultTask, dependsOn: setup) + if (node.javaVersion != null) { + BuildPlugin.requireJavaHome(start, node.javaVersion) + } start.doLast(elasticsearchRunner) return start } diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy index 1fc944eeec6eb..d5f77a8d52112 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy @@ -36,6 +36,9 @@ import static org.elasticsearch.gradle.BuildPlugin.getJavaHome * A container for the files and configuration associated with a single node in a test cluster. */ class NodeInfo { + /** Gradle project this node is part of */ + Project project + /** common configuration for all nodes, including this one */ ClusterConfiguration config @@ -84,6 +87,9 @@ class NodeInfo { /** directory to install plugins from */ File pluginsTmpDir + /** Major version of java this node runs with, or {@code null} if using the runtime java version */ + Integer javaVersion + /** environment variables to start the node with */ Map env @@ -109,6 +115,7 @@ class NodeInfo { NodeInfo(ClusterConfiguration config, int nodeNum, Project project, String prefix, Version nodeVersion, File sharedDir) { this.config = config this.nodeNum = nodeNum + this.project = project this.sharedDir = sharedDir if (config.clusterName != null) { clusterName = config.clusterName @@ -165,12 +172,11 @@ class NodeInfo { args.add("${esScript}") } + if (nodeVersion.before("6.2.0")) { - env = ['JAVA_HOME': "${-> getJavaHome(project, 8, "JAVA8_HOME must be set to run BWC tests against [" + nodeVersion + "]")}"] + javaVersion = 8 } else if (nodeVersion.onOrAfter("6.2.0") && nodeVersion.before("6.3.0")) { - env = ['JAVA_HOME': "${-> getJavaHome(project, 9, "JAVA9_HOME must be set to run BWC tests against [" + nodeVersion + "]")}"] - } else { - env = ['JAVA_HOME': (String) project.runtimeJavaHome] + javaVersion = 9 } args.addAll("-E", "node.portsfile=true") @@ -182,7 +188,7 @@ class NodeInfo { // in the cluster-specific options esJavaOpts = String.join(" ", "-ea", "-esa", esJavaOpts) } - env.put('ES_JAVA_OPTS', esJavaOpts) + env = ['ES_JAVA_OPTS': esJavaOpts] for (Map.Entry property : System.properties.entrySet()) { if (property.key.startsWith('tests.es.')) { args.add("-E") @@ -242,6 +248,11 @@ class NodeInfo { return Native.toString(shortPath).substring(4) } + /** Return the java home used by this node. */ + String getJavaHome() { + return javaVersion == null ? project.runtimeJavaHome : project.javaVersions.get(javaVersion) + } + /** Returns debug string for the command that started this node. */ String getCommandString() { String esCommandString = "\nNode ${nodeNum} configuration:\n" @@ -249,6 +260,7 @@ class NodeInfo { esCommandString += "| cwd: ${cwd}\n" esCommandString += "| command: ${executable} ${args.join(' ')}\n" esCommandString += '| environment:\n' + esCommandString += "| JAVA_HOME: ${javaHome}\n" env.each { k, v -> esCommandString += "| ${k}: ${v}\n" } if (config.daemonize) { esCommandString += "|\n| [${wrapperScript.name}]\n" diff --git a/distribution/bwc/build.gradle b/distribution/bwc/build.gradle index 48b84b4036240..8e29ff6011006 100644 --- a/distribution/bwc/build.gradle +++ b/distribution/bwc/build.gradle @@ -147,9 +147,9 @@ subprojects { workingDir = checkoutDir if (["5.6", "6.0", "6.1"].contains(bwcBranch)) { // we are building branches that are officially built with JDK 8, push JAVA8_HOME to JAVA_HOME for these builds - environment('JAVA_HOME', "${-> getJavaHome(project, 8, "JAVA8_HOME is required to build BWC versions for BWC branch [" + bwcBranch + "]")}") + environment('JAVA_HOME', getJavaHome(it, 8)) } else if ("6.2".equals(bwcBranch)) { - environment('JAVA_HOME', "${-> getJavaHome(project, 9, "JAVA9_HOME is required to build BWC versions for BWC branch [" + bwcBranch + "]")}") + environment('JAVA_HOME', getJavaHome(it, 9)) } else { environment('JAVA_HOME', project.compilerJavaHome) } diff --git a/qa/reindex-from-old/build.gradle b/qa/reindex-from-old/build.gradle index c4b4927a4a2b1..8da714dd6278a 100644 --- a/qa/reindex-from-old/build.gradle +++ b/qa/reindex-from-old/build.gradle @@ -77,7 +77,7 @@ if (Os.isFamily(Os.FAMILY_WINDOWS)) { dependsOn unzip executable = new File(project.runtimeJavaHome, 'bin/java') env 'CLASSPATH', "${ -> project.configurations.oldesFixture.asPath }" - env 'JAVA_HOME', "${-> getJavaHome(project, 7, "JAVA7_HOME must be set to run reindex-from-old")}" + env 'JAVA_HOME', getJavaHome(it, 7) args 'oldes.OldElasticsearch', baseDir, unzip.temporaryDir, From 00d88a5d3e334ca201dead987091312e4008f6c9 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 19 Apr 2018 11:02:49 -0700 Subject: [PATCH 09/11] Fix incorrect references to 'zero_terms_docs' in query parsing error messages. (#29599) --- .../org/elasticsearch/index/query/MatchQueryBuilder.java | 8 ++++---- .../index/query/MultiMatchQueryBuilder.java | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java index 3895aeab0f3da..74ccf423ffbfb 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java @@ -507,14 +507,14 @@ public static MatchQueryBuilder fromXContent(XContentParser parser) throws IOExc } else if (CUTOFF_FREQUENCY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { cutOffFrequency = parser.floatValue(); } else if (ZERO_TERMS_QUERY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { - String zeroTermsDocs = parser.text(); - if ("none".equalsIgnoreCase(zeroTermsDocs)) { + String zeroTermsValue = parser.text(); + if ("none".equalsIgnoreCase(zeroTermsValue)) { zeroTermsQuery = MatchQuery.ZeroTermsQuery.NONE; - } else if ("all".equalsIgnoreCase(zeroTermsDocs)) { + } else if ("all".equalsIgnoreCase(zeroTermsValue)) { zeroTermsQuery = MatchQuery.ZeroTermsQuery.ALL; } else { throw new ParsingException(parser.getTokenLocation(), - "Unsupported zero_terms_docs value [" + zeroTermsDocs + "]"); + "Unsupported zero_terms_query value [" + zeroTermsValue + "]"); } } else if (AbstractQueryBuilder.NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { queryName = parser.text(); diff --git a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java index e56fd44f5b856..156e6cca48f28 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java @@ -684,13 +684,14 @@ public static MultiMatchQueryBuilder fromXContent(XContentParser parser) throws } else if (LENIENT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { lenient = parser.booleanValue(); } else if (ZERO_TERMS_QUERY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { - String zeroTermsDocs = parser.text(); - if ("none".equalsIgnoreCase(zeroTermsDocs)) { + String zeroTermsValue = parser.text(); + if ("none".equalsIgnoreCase(zeroTermsValue)) { zeroTermsQuery = MatchQuery.ZeroTermsQuery.NONE; - } else if ("all".equalsIgnoreCase(zeroTermsDocs)) { + } else if ("all".equalsIgnoreCase(zeroTermsValue)) { zeroTermsQuery = MatchQuery.ZeroTermsQuery.ALL; } else { - throw new ParsingException(parser.getTokenLocation(), "Unsupported zero_terms_docs value [" + zeroTermsDocs + "]"); + throw new ParsingException(parser.getTokenLocation(), + "Unsupported zero_terms_query value [" + zeroTermsValue + "]"); } } else if (AbstractQueryBuilder.NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { queryName = parser.text(); From b9e1a002139a3fb550dfa68643efc6566ae95320 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 19 Apr 2018 11:25:27 -0700 Subject: [PATCH 10/11] Add support to match_phrase query for zero_terms_query. (#29598) --- .../query-dsl/match-phrase-query.asciidoc | 2 + .../index/query/MatchPhraseQueryBuilder.java | 49 +++++++++++- .../query/MatchPhraseQueryBuilderTests.java | 37 ++++++++- .../index/search/MatchPhraseQueryIT.java | 77 +++++++++++++++++++ 4 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/index/search/MatchPhraseQueryIT.java diff --git a/docs/reference/query-dsl/match-phrase-query.asciidoc b/docs/reference/query-dsl/match-phrase-query.asciidoc index 943d0e84d36db..1f4b19eedc132 100644 --- a/docs/reference/query-dsl/match-phrase-query.asciidoc +++ b/docs/reference/query-dsl/match-phrase-query.asciidoc @@ -39,3 +39,5 @@ GET /_search } -------------------------------------------------- // CONSOLE + +This query also accepts `zero_terms_query`, as explained in <>. diff --git a/server/src/main/java/org/elasticsearch/index/query/MatchPhraseQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MatchPhraseQueryBuilder.java index 03a1f78289409..5d05c4e92e519 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MatchPhraseQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MatchPhraseQueryBuilder.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.Query; +import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; @@ -28,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.search.MatchQuery; +import org.elasticsearch.index.search.MatchQuery.ZeroTermsQuery; import java.io.IOException; import java.util.Objects; @@ -39,6 +41,7 @@ public class MatchPhraseQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "match_phrase"; public static final ParseField SLOP_FIELD = new ParseField("slop"); + public static final ParseField ZERO_TERMS_QUERY_FIELD = new ParseField("zero_terms_query"); private final String fieldName; @@ -48,6 +51,8 @@ public class MatchPhraseQueryBuilder extends AbstractQueryBuilder { @@ -68,6 +71,11 @@ protected MatchPhraseQueryBuilder doCreateTestQueryBuilder() { if (randomBoolean()) { matchQuery.slop(randomIntBetween(0, 10)); } + + if (randomBoolean()) { + matchQuery.zeroTermsQuery(randomFrom(ZeroTermsQuery.ALL, ZeroTermsQuery.NONE)); + } + return matchQuery; } @@ -88,6 +96,12 @@ protected Map getAlternateVersions() { @Override protected void doAssertLuceneQuery(MatchPhraseQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { assertThat(query, notNullValue()); + + if (query instanceof MatchAllDocsQuery) { + assertThat(queryBuilder.zeroTermsQuery(), equalTo(ZeroTermsQuery.ALL)); + return; + } + assertThat(query, either(instanceOf(BooleanQuery.class)).or(instanceOf(PhraseQuery.class)) .or(instanceOf(TermQuery.class)).or(instanceOf(PointRangeQuery.class)) .or(instanceOf(IndexOrDocValuesQuery.class)).or(instanceOf(MatchNoDocsQuery.class))); @@ -108,7 +122,7 @@ public void testBadAnalyzer() throws IOException { assertThat(e.getMessage(), containsString("analyzer [bogusAnalyzer] not found")); } - public void testPhraseMatchQuery() throws IOException { + public void testFromSimpleJson() throws IOException { String json1 = "{\n" + " \"match_phrase\" : {\n" + " \"message\" : \"this is a test\"\n" + @@ -120,6 +134,7 @@ public void testPhraseMatchQuery() throws IOException { " \"message\" : {\n" + " \"query\" : \"this is a test\",\n" + " \"slop\" : 0,\n" + + " \"zero_terms_query\" : \"NONE\",\n" + " \"boost\" : 1.0\n" + " }\n" + " }\n" + @@ -128,6 +143,26 @@ public void testPhraseMatchQuery() throws IOException { checkGeneratedJson(expected, qb); } + public void testFromJson() throws IOException { + String json = "{\n" + + " \"match_phrase\" : {\n" + + " \"message\" : {\n" + + " \"query\" : \"this is a test\",\n" + + " \"slop\" : 2,\n" + + " \"zero_terms_query\" : \"ALL\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}"; + + MatchPhraseQueryBuilder parsed = (MatchPhraseQueryBuilder) parseQuery(json); + checkGeneratedJson(json, parsed); + + assertEquals(json, "this is a test", parsed.value()); + assertEquals(json, 2, parsed.slop()); + assertEquals(json, ZeroTermsQuery.ALL, parsed.zeroTermsQuery()); + } + public void testParseFailsWithMultipleFields() throws IOException { String json = "{\n" + " \"match_phrase\" : {\n" + diff --git a/server/src/test/java/org/elasticsearch/index/search/MatchPhraseQueryIT.java b/server/src/test/java/org/elasticsearch/index/search/MatchPhraseQueryIT.java new file mode 100644 index 0000000000000..ebd0bf0460aeb --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/search/MatchPhraseQueryIT.java @@ -0,0 +1,77 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.search; + +import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder; +import org.elasticsearch.action.index.IndexRequestBuilder; +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.query.MatchPhraseQueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.search.MatchQuery.ZeroTermsQuery; +import org.elasticsearch.test.ESIntegTestCase; +import org.junit.Before; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutionException; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; + +public class MatchPhraseQueryIT extends ESIntegTestCase { + private static final String INDEX = "test"; + + @Before + public void setUp() throws Exception { + super.setUp(); + CreateIndexRequestBuilder createIndexRequest = prepareCreate(INDEX).setSettings( + Settings.builder() + .put(indexSettings()) + .put("index.analysis.analyzer.standard_stopwords.type", "standard") + .putList("index.analysis.analyzer.standard_stopwords.stopwords", "of", "the", "who")); + assertAcked(createIndexRequest); + ensureGreen(); + } + + public void testZeroTermsQuery() throws ExecutionException, InterruptedException { + List indexRequests = getIndexRequests(); + indexRandom(true, false, indexRequests); + + MatchPhraseQueryBuilder baseQuery = QueryBuilders.matchPhraseQuery("name", "the who") + .analyzer("standard_stopwords"); + + MatchPhraseQueryBuilder matchNoneQuery = baseQuery.zeroTermsQuery(ZeroTermsQuery.NONE); + SearchResponse matchNoneResponse = client().prepareSearch(INDEX).setQuery(matchNoneQuery).get(); + assertHitCount(matchNoneResponse, 0L); + + MatchPhraseQueryBuilder matchAllQuery = baseQuery.zeroTermsQuery(ZeroTermsQuery.ALL); + SearchResponse matchAllResponse = client().prepareSearch(INDEX).setQuery(matchAllQuery).get(); + assertHitCount(matchAllResponse, 2L); + } + + + private List getIndexRequests() { + List requests = new ArrayList<>(); + requests.add(client().prepareIndex(INDEX, "band").setSource("name", "the beatles")); + requests.add(client().prepareIndex(INDEX, "band").setSource("name", "led zeppelin")); + return requests; + } +} From a28c0d227193af43c814ae1a38cd5245354f09c1 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 19 Apr 2018 15:10:19 -0400 Subject: [PATCH 11/11] Remove extra spaces from changelog This commit removes to extra spaces at the end of lines in the changelog. --- docs/CHANGELOG.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index 9c6e36a2e1d0d..0654801f8cb26 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -13,7 +13,7 @@ === Deprecations -=== New Features +=== New Features === Enhancements @@ -25,7 +25,7 @@ == Elasticsearch version 6.3.0 -=== New Features +=== New Features === Enhancements