From 6bb9e3e78bd0cf22fc0033151bea50016139d7f0 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel <104654647+gashutos@users.noreply.github.com> Date: Wed, 15 Feb 2023 23:17:16 +0530 Subject: [PATCH] Enable numeric sort optimisation for few numerical sort types (#6321) This commit restores the sort optimization to use BKD to skip non-competitive docs for numeric types whose BYTES size match between the BKD leaf and doc values encoding. For now this is only LONG, DOUBLE, DATE, and DATE_NANOSECONDS as the remaining NumericTypes use 64bit docvalue encoding while the BKD uses smaller byte encoded space. This also updates the QueryPhase to remove the long time unnecessary in order doc id check and minDoc boolean query for skipping non-competitive docs that is handled by all Lucene 7.0+ sorted indexes. Existing tests are updated. Signed-off-by: Nicholas Walter Knize Signed-off-by: gashutos Co-authored-by: Nicholas Walter Knize Co-authored-by: Chaitanya Gohel --- CHANGELOG.md | 1 + .../search/query/QueryPhaseTests.java | 2 +- .../search/query/QueryProfilePhaseTests.java | 18 ++- .../fielddata/IndexNumericFieldData.java | 115 ++++++++++++++++-- .../opensearch/search/query/QueryPhase.java | 31 +---- .../search/query/QueryPhaseTests.java | 2 +- .../search/query/QueryProfilePhaseTests.java | 19 ++- 7 files changed, 123 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad0492dd08307..0989db87c1673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Segment Replication] Fix for peer recovery ([#5344](https://github.com/opensearch-project/OpenSearch/pull/5344)) - Fix weighted shard routing state across search requests([#6004](https://github.com/opensearch-project/OpenSearch/pull/6004)) - [Segment Replication] Fix bug where inaccurate sequence numbers are sent during replication ([#6122](https://github.com/opensearch-project/OpenSearch/pull/6122)) +- Enable numeric sort optimisation for few numerical sort types ([#6321](https://github.com/opensearch-project/OpenSearch/pull/6321)) ### Security diff --git a/sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryPhaseTests.java b/sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryPhaseTests.java index 2768a38cf673d..2b4ad8f0a26bd 100644 --- a/sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryPhaseTests.java +++ b/sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryPhaseTests.java @@ -363,6 +363,7 @@ public void testInOrderScrollOptimization() throws Exception { ScrollContext scrollContext = new ScrollContext(); TestSearchContext context = new TestSearchContext(null, indexShard, newContextSearcher(reader, executor), scrollContext); context.parsedQuery(new ParsedQuery(new MatchAllDocsQuery())); + context.sort(new SortAndFormats(sort, new DocValueFormat[] { DocValueFormat.RAW })); scrollContext.lastEmittedDoc = null; scrollContext.maxScore = Float.NaN; scrollContext.totalHits = null; @@ -379,7 +380,6 @@ public void testInOrderScrollOptimization() throws Exception { context.setSearcher(newEarlyTerminationContextSearcher(reader, size, executor)); QueryPhase.executeInternal(context.withCleanQueryResult(), queryPhaseSearcher); assertThat(context.queryResult().topDocs().topDocs.totalHits.value, equalTo((long) numDocs)); - assertThat(context.terminateAfter(), equalTo(size)); assertThat(context.queryResult().getTotalHits().value, equalTo((long) numDocs)); assertThat(context.queryResult().topDocs().topDocs.scoreDocs[0].doc, greaterThanOrEqualTo(size)); reader.close(); diff --git a/sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java b/sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java index 36c20d029e997..76686e9052d76 100644 --- a/sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java +++ b/sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java @@ -319,6 +319,7 @@ public void testInOrderScrollOptimization() throws Exception { ScrollContext scrollContext = new ScrollContext(); TestSearchContext context = new TestSearchContext(null, indexShard, newContextSearcher(reader, executor), scrollContext); context.parsedQuery(new ParsedQuery(new MatchAllDocsQuery())); + context.sort(new SortAndFormats(sort, new DocValueFormat[] { DocValueFormat.RAW })); scrollContext.lastEmittedDoc = null; scrollContext.maxScore = Float.NaN; scrollContext.totalHits = null; @@ -331,10 +332,10 @@ public void testInOrderScrollOptimization() throws Exception { assertNull(context.queryResult().terminatedEarly()); assertThat(context.terminateAfter(), equalTo(0)); assertThat(context.queryResult().getTotalHits().value, equalTo((long) numDocs)); - assertProfileData(context, "MatchAllDocsQuery", query -> { + assertProfileData(context, "ConstantScoreQuery", query -> { assertThat(query.getTimeBreakdown().keySet(), not(empty())); - assertThat(query.getTimeBreakdown().get("score"), greaterThan(0L)); - assertThat(query.getTimeBreakdown().get("score_count"), greaterThan(0L)); + assertThat(query.getTimeBreakdown().get("score"), equalTo(0L)); + assertThat(query.getTimeBreakdown().get("score_count"), equalTo(0L)); assertThat(query.getTimeBreakdown().get("create_weight"), greaterThan(0L)); assertThat(query.getTimeBreakdown().get("create_weight_count"), equalTo(1L)); }, collector -> { @@ -346,13 +347,12 @@ public void testInOrderScrollOptimization() throws Exception { context.setSearcher(newEarlyTerminationContextSearcher(reader, size, executor)); QueryPhase.executeInternal(context.withCleanQueryResult().withProfilers(), queryPhaseSearcher); assertThat(context.queryResult().topDocs().topDocs.totalHits.value, equalTo((long) numDocs)); - assertThat(context.terminateAfter(), equalTo(size)); assertThat(context.queryResult().getTotalHits().value, equalTo((long) numDocs)); assertThat(context.queryResult().topDocs().topDocs.scoreDocs[0].doc, greaterThanOrEqualTo(size)); assertProfileData(context, "ConstantScoreQuery", query -> { assertThat(query.getTimeBreakdown().keySet(), not(empty())); - assertThat(query.getTimeBreakdown().get("score"), greaterThan(0L)); - assertThat(query.getTimeBreakdown().get("score_count"), greaterThan(0L)); + assertThat(query.getTimeBreakdown().get("score"), equalTo(0L)); + assertThat(query.getTimeBreakdown().get("score_count"), equalTo(0L)); assertThat(query.getTimeBreakdown().get("create_weight"), greaterThan(0L)); assertThat(query.getTimeBreakdown().get("create_weight_count"), equalTo(1L)); assertThat(query.getProfiledChildren().get(0).getTimeBreakdown().get("score"), equalTo(0L)); @@ -360,11 +360,9 @@ public void testInOrderScrollOptimization() throws Exception { assertThat(query.getProfiledChildren().get(0).getTimeBreakdown().get("create_weight"), greaterThan(0L)); assertThat(query.getProfiledChildren().get(0).getTimeBreakdown().get("create_weight_count"), equalTo(1L)); }, collector -> { - assertThat(collector.getReason(), equalTo("search_terminate_after_count")); + assertThat(collector.getReason(), equalTo("search_top_hits")); assertThat(collector.getTime(), greaterThan(0L)); - assertThat(collector.getProfiledChildren(), hasSize(1)); - assertThat(collector.getProfiledChildren().get(0).getReason(), equalTo("search_top_hits")); - assertThat(collector.getProfiledChildren().get(0).getTime(), greaterThan(0L)); + assertThat(collector.getProfiledChildren(), hasSize(0)); }); reader.close(); diff --git a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java index 02748edfb4f83..6fe79565a51e1 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java @@ -65,16 +65,76 @@ public abstract class IndexNumericFieldData implements IndexFieldDataquery returns docs in index order (internal doc ids). - * @param query The query to execute - * @param sf The query sort - */ - private static boolean returnsDocsInOrder(Query query, SortAndFormats sf) { - if (sf == null || Sort.RELEVANCE.equals(sf.sort)) { - // sort by score - // queries that return constant scores will return docs in index - // order since Lucene tie-breaks on the doc id - return query.getClass() == ConstantScoreQuery.class || query.getClass() == MatchAllDocsQuery.class; - } else { - return Sort.INDEXORDER.equals(sf.sort); - } - } - /** * Returns whether collection within the provided reader can be early-terminated if it sorts * with sortAndFormats. diff --git a/server/src/test/java/org/opensearch/search/query/QueryPhaseTests.java b/server/src/test/java/org/opensearch/search/query/QueryPhaseTests.java index 9b2edccff82ee..18225ba887416 100644 --- a/server/src/test/java/org/opensearch/search/query/QueryPhaseTests.java +++ b/server/src/test/java/org/opensearch/search/query/QueryPhaseTests.java @@ -342,6 +342,7 @@ public void testInOrderScrollOptimization() throws Exception { ScrollContext scrollContext = new ScrollContext(); TestSearchContext context = new TestSearchContext(null, indexShard, newContextSearcher(reader), scrollContext); context.parsedQuery(new ParsedQuery(new MatchAllDocsQuery())); + context.sort(new SortAndFormats(sort, new DocValueFormat[] { DocValueFormat.RAW })); scrollContext.lastEmittedDoc = null; scrollContext.maxScore = Float.NaN; scrollContext.totalHits = null; @@ -358,7 +359,6 @@ public void testInOrderScrollOptimization() throws Exception { context.setSearcher(newEarlyTerminationContextSearcher(reader, size)); QueryPhase.executeInternal(context.withCleanQueryResult()); assertThat(context.queryResult().topDocs().topDocs.totalHits.value, equalTo((long) numDocs)); - assertThat(context.terminateAfter(), equalTo(size)); assertThat(context.queryResult().getTotalHits().value, equalTo((long) numDocs)); assertThat(context.queryResult().topDocs().topDocs.scoreDocs[0].doc, greaterThanOrEqualTo(size)); reader.close(); diff --git a/server/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java b/server/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java index 1b168e7d5b16c..bfec445d683df 100644 --- a/server/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java +++ b/server/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java @@ -289,6 +289,7 @@ public void testInOrderScrollOptimization() throws Exception { ScrollContext scrollContext = new ScrollContext(); TestSearchContext context = new TestSearchContext(null, indexShard, newContextSearcher(reader), scrollContext); context.parsedQuery(new ParsedQuery(new MatchAllDocsQuery())); + context.sort(new SortAndFormats(sort, new DocValueFormat[] { DocValueFormat.RAW })); scrollContext.lastEmittedDoc = null; scrollContext.maxScore = Float.NaN; scrollContext.totalHits = null; @@ -299,12 +300,11 @@ public void testInOrderScrollOptimization() throws Exception { QueryPhase.executeInternal(context.withCleanQueryResult().withProfilers()); assertThat(context.queryResult().topDocs().topDocs.totalHits.value, equalTo((long) numDocs)); assertNull(context.queryResult().terminatedEarly()); - assertThat(context.terminateAfter(), equalTo(0)); assertThat(context.queryResult().getTotalHits().value, equalTo((long) numDocs)); - assertProfileData(context, "MatchAllDocsQuery", query -> { + assertProfileData(context, "ConstantScoreQuery", query -> { assertThat(query.getTimeBreakdown().keySet(), not(empty())); - assertThat(query.getTimeBreakdown().get("score"), greaterThan(0L)); - assertThat(query.getTimeBreakdown().get("score_count"), greaterThan(0L)); + assertThat(query.getTimeBreakdown().get("score"), equalTo(0L)); + assertThat(query.getTimeBreakdown().get("score_count"), equalTo(0L)); assertThat(query.getTimeBreakdown().get("create_weight"), greaterThan(0L)); assertThat(query.getTimeBreakdown().get("create_weight_count"), equalTo(1L)); }, collector -> { @@ -316,13 +316,12 @@ public void testInOrderScrollOptimization() throws Exception { context.setSearcher(newEarlyTerminationContextSearcher(reader, size)); QueryPhase.executeInternal(context.withCleanQueryResult().withProfilers()); assertThat(context.queryResult().topDocs().topDocs.totalHits.value, equalTo((long) numDocs)); - assertThat(context.terminateAfter(), equalTo(size)); assertThat(context.queryResult().getTotalHits().value, equalTo((long) numDocs)); assertThat(context.queryResult().topDocs().topDocs.scoreDocs[0].doc, greaterThanOrEqualTo(size)); assertProfileData(context, "ConstantScoreQuery", query -> { assertThat(query.getTimeBreakdown().keySet(), not(empty())); - assertThat(query.getTimeBreakdown().get("score"), greaterThan(0L)); - assertThat(query.getTimeBreakdown().get("score_count"), greaterThan(0L)); + assertThat(query.getTimeBreakdown().get("score"), equalTo(0L)); + assertThat(query.getTimeBreakdown().get("score_count"), equalTo(0L)); assertThat(query.getTimeBreakdown().get("create_weight"), greaterThan(0L)); assertThat(query.getTimeBreakdown().get("create_weight_count"), equalTo(1L)); assertThat(query.getProfiledChildren().get(0).getTimeBreakdown().get("score"), equalTo(0L)); @@ -330,11 +329,9 @@ public void testInOrderScrollOptimization() throws Exception { assertThat(query.getProfiledChildren().get(0).getTimeBreakdown().get("create_weight"), greaterThan(0L)); assertThat(query.getProfiledChildren().get(0).getTimeBreakdown().get("create_weight_count"), equalTo(1L)); }, collector -> { - assertThat(collector.getReason(), equalTo("search_terminate_after_count")); + assertThat(collector.getReason(), equalTo("search_top_hits")); assertThat(collector.getTime(), greaterThan(0L)); - assertThat(collector.getProfiledChildren(), hasSize(1)); - assertThat(collector.getProfiledChildren().get(0).getReason(), equalTo("search_top_hits")); - assertThat(collector.getProfiledChildren().get(0).getTime(), greaterThan(0L)); + assertThat(collector.getProfiledChildren(), hasSize(0)); }); reader.close();