From cff74ff022899fe2ae9196fa6b447189dc9c89d4 Mon Sep 17 00:00:00 2001 From: Asif Bashar Date: Thu, 31 Jul 2025 08:34:02 -0700 Subject: [PATCH 001/442] [BUG] Repository s3 plugin test S3RepositoryThirdPartyTests.java does not have an option to override path_style_access and endpoint #18796 (#18855) * Repository s3 plugin test S3RepositoryThirdPartyTests.java does not have an option to override pathStyle and endpoint Signed-off-by: Asif Bashar * Repository s3 plugin test S3RepositoryThirdPartyTests.java does not have an option to override pathStyle and endpoint Signed-off-by: Asif Bashar * Repository s3 plugin test S3RepositoryThirdPartyTests.java does not have an option to override pathStyle and endpoint Signed-off-by: Asif Bashar * Repository s3 plugin test S3RepositoryThirdPartyTests.java does not have an option to override pathStyle and endpoint Signed-off-by: Asif Bashar --------- Signed-off-by: Asif Bashar --- plugins/repository-s3/README.md | 4 ++++ plugins/repository-s3/build.gradle | 11 +++++++++++ .../repositories/s3/S3RepositoryThirdPartyTests.java | 3 +++ 3 files changed, 18 insertions(+) diff --git a/plugins/repository-s3/README.md b/plugins/repository-s3/README.md index 03007e03b633e..9bd2fb4bed2e1 100644 --- a/plugins/repository-s3/README.md +++ b/plugins/repository-s3/README.md @@ -23,3 +23,7 @@ Integration tests require several environment variables. ``` AWS_REGION=us-west-2 amazon_s3_access_key=$AWS_ACCESS_KEY_ID amazon_s3_secret_key=$AWS_SECRET_ACCESS_KEY amazon_s3_base_path=path amazon_s3_bucket=dblock-opensearch ./gradlew :plugins:repository-s3:s3ThirdPartyTest ``` +Optional environment variables: + +- `amazon_s3_path_style_access`: Possible values true or false. Default is false. +- `amazon_s3_endpoint`: s3 custom endpoint url if aws s3 default endpoint is not being used. diff --git a/plugins/repository-s3/build.gradle b/plugins/repository-s3/build.gradle index 25d910052b9a0..b1a83565f0e87 100644 --- a/plugins/repository-s3/build.gradle +++ b/plugins/repository-s3/build.gradle @@ -161,6 +161,9 @@ String s3PermanentSecretKey = System.getenv("amazon_s3_secret_key") String s3PermanentBucket = System.getenv("amazon_s3_bucket") String s3PermanentBasePath = System.getenv("amazon_s3_base_path") String s3PermanentRegion = System.getenv("amazon_s3_region") +String s3Endpoint = System.getenv("amazon_s3_endpoint") +String s3PathStyleAccess = System.getenv("amazon_s3_path_style_access") + String s3TemporaryAccessKey = System.getenv("amazon_s3_access_key_temporary") String s3TemporarySecretKey = System.getenv("amazon_s3_secret_key_temporary") @@ -419,6 +422,14 @@ TaskProvider s3ThirdPartyTest = tasks.register("s3ThirdPartyTest", Test) { if (useFixture) { nonInputProperties.systemProperty 'test.s3.endpoint', "${-> fixtureAddress('minio-fixture', 'minio-fixture', '9000') }" } + else { + if (s3Endpoint != null) { + systemProperty 'test.s3.endpoint', s3Endpoint + } + } + if (s3PathStyleAccess) { + systemProperty 'test.s3.path_style_access', s3PathStyleAccess + } } tasks.named("check").configure { dependsOn(s3ThirdPartyTest) } diff --git a/plugins/repository-s3/src/internalClusterTest/java/org/opensearch/repositories/s3/S3RepositoryThirdPartyTests.java b/plugins/repository-s3/src/internalClusterTest/java/org/opensearch/repositories/s3/S3RepositoryThirdPartyTests.java index 7db9a0d3ba790..decee71d420f3 100644 --- a/plugins/repository-s3/src/internalClusterTest/java/org/opensearch/repositories/s3/S3RepositoryThirdPartyTests.java +++ b/plugins/repository-s3/src/internalClusterTest/java/org/opensearch/repositories/s3/S3RepositoryThirdPartyTests.java @@ -94,6 +94,9 @@ protected void createRepository(String repoName) { .put("region", System.getProperty("test.s3.region", "us-west-2")) .put("base_path", System.getProperty("test.s3.base", "testpath")); final String endpoint = System.getProperty("test.s3.endpoint"); + final boolean pathStyleAccess = Boolean.parseBoolean(System.getProperty("test.s3.path_style_access")); + settings.put("path_style_access", pathStyleAccess); + if (endpoint != null) { settings.put("endpoint", endpoint); } else { From 4b90da1b3ab9eec2da1c1216501e68d4c5110311 Mon Sep 17 00:00:00 2001 From: Prudhvi Godithi Date: Thu, 31 Jul 2025 09:08:01 -0700 Subject: [PATCH 002/442] Disable Approximation when dealing with multiple sorts (#18763) * Approximation Framework bug fix Signed-off-by: Prudhvi Godithi * Add tests Signed-off-by: Prudhvi Godithi * Fix spacing Signed-off-by: Prudhvi Godithi --------- Signed-off-by: Prudhvi Godithi --- CHANGELOG.md | 1 + .../approximate/ApproximateMatchAllQuery.java | 3 ++ .../ApproximatePointRangeQuery.java | 3 ++ .../ApproximatePointRangeQueryTests.java | 46 +++++++++++++++++++ 4 files changed, 53 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02c45d0e36057..0f713e36f239a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Pass index settings to system ingest processor factories. ([#18708](https://github.com/opensearch-project/OpenSearch/pull/18708)) - Include named queries from rescore contexts in matched_queries array ([#18697](https://github.com/opensearch-project/OpenSearch/pull/18697)) - Add the configurable limit on rule cardinality ([#18663](https://github.com/opensearch-project/OpenSearch/pull/18663)) +- Disable approximation framework when dealing with multiple sorts ([#18763](https://github.com/opensearch-project/OpenSearch/pull/18763)) - [Experimental] Start in "clusterless" mode if a clusterless ClusterPlugin is loaded ([#18479](https://github.com/opensearch-project/OpenSearch/pull/18479)) - [Star-Tree] Add star-tree search related stats ([#18707](https://github.com/opensearch-project/OpenSearch/pull/18707)) - Add support for plugins to profile information ([#18656](https://github.com/opensearch-project/OpenSearch/pull/18656)) diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximateMatchAllQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximateMatchAllQuery.java index 62557bf21eee9..bcd40095dbc8f 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximateMatchAllQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximateMatchAllQuery.java @@ -42,6 +42,9 @@ protected boolean canApproximate(SearchContext context) { } if (context.request() != null && context.request().source() != null && context.innerHits().getInnerHits().isEmpty()) { + if (context.request().source().sorts() != null && context.request().source().sorts().size() > 1) { + return false; + } FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(context.request().source()); if (primarySortField != null && primarySortField.missing() == null diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java index e6d11d02eacf6..adecb8c89ef82 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java @@ -447,6 +447,9 @@ public boolean canApproximate(SearchContext context) { this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo()) + 1); } if (context.request() != null && context.request().source() != null) { + if (context.request().source().sorts() != null && context.request().source().sorts().size() > 1) { + return false; + } FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(context.request().source()); if (primarySortField != null) { if (!primarySortField.fieldName().equals(pointRangeQuery.getField())) { diff --git a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java index a62a5568cc0bb..e7ef69b2ad8c6 100644 --- a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java +++ b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java @@ -33,7 +33,10 @@ import org.opensearch.common.time.DateFormatter; import org.opensearch.common.time.DateMathParser; import org.opensearch.index.mapper.DateFieldMapper.DateFieldType; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.search.internal.SearchContext; +import org.opensearch.search.internal.ShardSearchRequest; +import org.opensearch.search.sort.FieldSortBuilder; import org.opensearch.search.sort.SortOrder; import org.opensearch.test.OpenSearchTestCase; @@ -1142,4 +1145,47 @@ private void verifyRangeQueries( approxDocsAsc.scoreDocs.length ); } + + public void testApproximateWithSort() { + long lower = RandomNumbers.randomLongBetween(random(), 0, 100); + long upper = RandomNumbers.randomLongBetween(random(), lower + 10, lower + 1000); + ApproximatePointRangeQuery query = new ApproximatePointRangeQuery( + numericType.fieldName, + numericType.encode(lower), + numericType.encode(upper), + 1, + numericType.format + ); + // Test 1: Multiple sorts should prevent approximation + { + SearchContext mockContext = mock(SearchContext.class); + ShardSearchRequest mockRequest = mock(ShardSearchRequest.class); + SearchSourceBuilder source = new SearchSourceBuilder(); + source.sort(new FieldSortBuilder(numericType.fieldName).order(SortOrder.ASC)); + source.sort(new FieldSortBuilder("another_field").order(SortOrder.ASC)); + source.terminateAfter(SearchContext.DEFAULT_TERMINATE_AFTER); + when(mockContext.aggregations()).thenReturn(null); + when(mockContext.trackTotalHitsUpTo()).thenReturn(10000); + when(mockContext.from()).thenReturn(0); + when(mockContext.size()).thenReturn(10); + when(mockContext.request()).thenReturn(mockRequest); + when(mockRequest.source()).thenReturn(source); + assertFalse("Should not approximate with multiple sorts", query.canApproximate(mockContext)); + } + // Test 2: Single sort on the same field should allow approximation + { + SearchContext mockContext = mock(SearchContext.class); + ShardSearchRequest mockRequest = mock(ShardSearchRequest.class); + SearchSourceBuilder source = new SearchSourceBuilder(); + source.sort(new FieldSortBuilder(numericType.fieldName).order(SortOrder.ASC)); + source.terminateAfter(SearchContext.DEFAULT_TERMINATE_AFTER); + when(mockContext.aggregations()).thenReturn(null); + when(mockContext.trackTotalHitsUpTo()).thenReturn(10000); + when(mockContext.from()).thenReturn(0); + when(mockContext.size()).thenReturn(10); + when(mockContext.request()).thenReturn(mockRequest); + when(mockRequest.source()).thenReturn(source); + assertTrue("Should approximate with single sort on same field", query.canApproximate(mockContext)); + } + } } From 9b22c9bf1e6a5c89177ec35e81dad987909efb52 Mon Sep 17 00:00:00 2001 From: Karen X Date: Thu, 31 Jul 2025 16:40:14 -0400 Subject: [PATCH 003/442] Upgrade to protobufs 0.6.0 and clean up deprecated TermQueryProtoUtils code (#18880) Signed-off-by: Karen Xu --- CHANGELOG.md | 1 + plugins/transport-grpc/build.gradle | 2 +- .../licenses/protobufs-0.4.0.jar.sha1 | 1 - .../licenses/protobufs-0.6.0.jar.sha1 | 1 + .../query/MatchAllQueryBuilderProtoUtils.java | 4 +- .../MatchNoneQueryBuilderProtoUtils.java | 4 +- .../query/TermQueryBuilderProtoUtils.java | 94 +-------------- .../query/TermsQueryBuilderProtoUtils.java | 4 +- ...tchAllQueryBuilderProtoConverterTests.java | 2 +- .../MatchAllQueryBuilderProtoUtilsTests.java | 4 +- ...chNoneQueryBuilderProtoConverterTests.java | 2 +- .../MatchNoneQueryBuilderProtoUtilsTests.java | 4 +- .../TermQueryBuilderProtoConverterTests.java | 2 +- .../TermQueryBuilderProtoUtilsTests.java | 114 ++++++------------ .../TermsQueryBuilderProtoConverterTests.java | 2 +- .../TermsQueryBuilderProtoUtilsTests.java | 4 +- 16 files changed, 55 insertions(+), 190 deletions(-) delete mode 100644 plugins/transport-grpc/licenses/protobufs-0.4.0.jar.sha1 create mode 100644 plugins/transport-grpc/licenses/protobufs-0.6.0.jar.sha1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f713e36f239a..bb136d0121d53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add support for Combined Fields query ([#18724](https://github.com/opensearch-project/OpenSearch/pull/18724)) - Make GRPC transport extensible to allow plugins to register and expose their own GRPC services ([#18516](https://github.com/opensearch-project/OpenSearch/pull/18516)) - Added approximation support for range queries with now in date field ([#18511](https://github.com/opensearch-project/OpenSearch/pull/18511)) +- Upgrade to protobufs 0.6.0 and clean up deprecated TermQueryProtoUtils code ([#18880](https://github.com/opensearch-project/OpenSearch/pull/18880)) ### Changed - Update Subject interface to use CheckedRunnable ([#18570](https://github.com/opensearch-project/OpenSearch/issues/18570)) diff --git a/plugins/transport-grpc/build.gradle b/plugins/transport-grpc/build.gradle index 5920ba83b6d0d..df87623dcc1e2 100644 --- a/plugins/transport-grpc/build.gradle +++ b/plugins/transport-grpc/build.gradle @@ -35,7 +35,7 @@ dependencies { implementation "io.grpc:grpc-stub:${versions.grpc}" implementation "io.grpc:grpc-util:${versions.grpc}" implementation "io.perfmark:perfmark-api:0.27.0" - implementation "org.opensearch:protobufs:0.4.0" + implementation "org.opensearch:protobufs:0.6.0" testImplementation project(':test:framework') } diff --git a/plugins/transport-grpc/licenses/protobufs-0.4.0.jar.sha1 b/plugins/transport-grpc/licenses/protobufs-0.4.0.jar.sha1 deleted file mode 100644 index 12f76199639f8..0000000000000 --- a/plugins/transport-grpc/licenses/protobufs-0.4.0.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -af2d6818dab60d54689122e57f3d3b8fb86cf67b \ No newline at end of file diff --git a/plugins/transport-grpc/licenses/protobufs-0.6.0.jar.sha1 b/plugins/transport-grpc/licenses/protobufs-0.6.0.jar.sha1 new file mode 100644 index 0000000000000..a02b4926d87f5 --- /dev/null +++ b/plugins/transport-grpc/licenses/protobufs-0.6.0.jar.sha1 @@ -0,0 +1 @@ +1675c5085e1376fd1a107b87f7e325944ab5b4dc \ No newline at end of file diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoUtils.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoUtils.java index 82babebf1cf66..98b33826ca46f 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoUtils.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoUtils.java @@ -36,8 +36,8 @@ protected static MatchAllQueryBuilder fromProto(MatchAllQuery matchAllQueryProto matchAllQueryBuilder.boost(matchAllQueryProto.getBoost()); } - if (matchAllQueryProto.hasName()) { - matchAllQueryBuilder.queryName(matchAllQueryProto.getName()); + if (matchAllQueryProto.hasUnderscoreName()) { + matchAllQueryBuilder.queryName(matchAllQueryProto.getUnderscoreName()); } return matchAllQueryBuilder; diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoUtils.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoUtils.java index 476b63daca906..06876fc53ea08 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoUtils.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoUtils.java @@ -38,8 +38,8 @@ protected static MatchNoneQueryBuilder fromProto(MatchNoneQuery matchNoneQueryPr matchNoneQueryBuilder.boost(matchNoneQueryProto.getBoost()); } - if (matchNoneQueryProto.hasName()) { - matchNoneQueryBuilder.queryName(matchNoneQueryProto.getName()); + if (matchNoneQueryProto.hasUnderscoreName()) { + matchNoneQueryBuilder.queryName(matchNoneQueryProto.getUnderscoreName()); } return matchNoneQueryBuilder; diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtils.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtils.java index cc896d703cb0e..7c1a2736b08d7 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtils.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtils.java @@ -14,8 +14,6 @@ import org.opensearch.protobufs.FieldValue; import org.opensearch.protobufs.TermQuery; -import java.util.Map; - /** * Utility class for converting TermQuery Protocol Buffers to OpenSearch objects. * This class provides methods to transform Protocol Buffer representations of term queries @@ -45,8 +43,8 @@ protected static TermQueryBuilder fromProto(TermQuery termQueryProto) { float boost = AbstractQueryBuilder.DEFAULT_BOOST; boolean caseInsensitive = TermQueryBuilder.DEFAULT_CASE_INSENSITIVITY; - if (termQueryProto.hasName()) { - queryName = termQueryProto.getName(); + if (termQueryProto.hasUnderscoreName()) { + queryName = termQueryProto.getUnderscoreName(); } if (termQueryProto.hasBoost()) { boost = termQueryProto.getBoost(); @@ -102,92 +100,4 @@ protected static TermQueryBuilder fromProto(TermQuery termQueryProto) { return termQuery; } - /** - * Converts a Protocol Buffer TermQuery map to an OpenSearch TermQueryBuilder. - * Similar to {@link TermQueryBuilder#fromXContent(XContentParser)}, this method - * parses the Protocol Buffer representation and creates a properly configured - * TermQueryBuilder with the appropriate field name, value, boost, query name, - * and case sensitivity settings. - * - * @param termQueryProto The map of field names to Protocol Buffer TermQuery objects - * @return A configured TermQueryBuilder instance - * @throws IllegalArgumentException if the term query map has more than one element, - * if the field value type is not supported, or if the term query field value is not recognized - * @deprecated Use {@link #fromProto(TermQuery)} instead for the new protobuf structure - */ - @Deprecated - protected static TermQueryBuilder fromProto(Map termQueryProto) { - String queryName = null; - String fieldName = null; - Object value = null; - float boost = AbstractQueryBuilder.DEFAULT_BOOST; - boolean caseInsensitive = TermQueryBuilder.DEFAULT_CASE_INSENSITIVITY; - - if (termQueryProto.size() > 1) { - throw new IllegalArgumentException("Term query can only have 1 element in the map"); - } - - for (Map.Entry entry : termQueryProto.entrySet()) { - - fieldName = entry.getKey(); - - TermQuery termQuery = entry.getValue(); - - if (termQuery.hasName()) { - queryName = termQuery.getName(); - } - if (termQuery.hasBoost()) { - boost = termQuery.getBoost(); - } - - FieldValue fieldValue = termQuery.getValue(); - - switch (fieldValue.getTypeCase()) { - case GENERAL_NUMBER: - switch (fieldValue.getGeneralNumber().getValueCase()) { - case INT32_VALUE: - value = fieldValue.getGeneralNumber().getInt32Value(); - break; - case INT64_VALUE: - value = fieldValue.getGeneralNumber().getInt64Value(); - break; - case FLOAT_VALUE: - value = fieldValue.getGeneralNumber().getFloatValue(); - break; - case DOUBLE_VALUE: - value = fieldValue.getGeneralNumber().getDoubleValue(); - break; - default: - throw new IllegalArgumentException( - "Unsupported general nunber type: " + fieldValue.getGeneralNumber().getValueCase() - ); - } - break; - case STRING_VALUE: - value = fieldValue.getStringValue(); - break; - case OBJECT_MAP: - value = ObjectMapProtoUtils.fromProto(fieldValue.getObjectMap()); - break; - case BOOL_VALUE: - value = fieldValue.getBoolValue(); - break; - default: - throw new IllegalArgumentException("TermQuery field value not recognized"); - } - - if (termQuery.hasCaseInsensitive()) { - caseInsensitive = termQuery.getCaseInsensitive(); - } - - } - TermQueryBuilder termQuery = new TermQueryBuilder(fieldName, value); - termQuery.boost(boost); - if (queryName != null) { - termQuery.queryName(queryName); - } - termQuery.caseInsensitive(caseInsensitive); - - return termQuery; - } } diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java index 34bea81af9699..8a1faf572f7ab 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java @@ -63,8 +63,8 @@ protected static TermsQueryBuilder fromProto(TermsQueryField termsQueryProto) { boost = termsQueryProto.getBoost(); } - if (termsQueryProto.hasName()) { - queryName = termsQueryProto.getName(); + if (termsQueryProto.hasUnderscoreName()) { + queryName = termsQueryProto.getUnderscoreName(); } // TODO: remove this parameter when backporting to under OS 2.17 diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverterTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverterTests.java index ab0e106e48b79..3d722655f6598 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverterTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverterTests.java @@ -34,7 +34,7 @@ public void testGetHandledQueryCase() { public void testFromProto() { // Create a QueryContainer with MatchAllQuery - MatchAllQuery matchAllQuery = MatchAllQuery.newBuilder().setBoost(2.0f).setName("test_query").build(); + MatchAllQuery matchAllQuery = MatchAllQuery.newBuilder().setBoost(2.0f).setUnderscoreName("test_query").build(); QueryContainer queryContainer = QueryContainer.newBuilder().setMatchAll(matchAllQuery).build(); // Convert the query diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoUtilsTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoUtilsTests.java index 186f54f09a51f..c2b0fb9fce2f0 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoUtilsTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoUtilsTests.java @@ -42,7 +42,7 @@ public void testFromProtoWithBoost() { public void testFromProtoWithName() { // Create a protobuf MatchAllQuery with name - MatchAllQuery matchAllQueryProto = MatchAllQuery.newBuilder().setName("test_query").build(); + MatchAllQuery matchAllQueryProto = MatchAllQuery.newBuilder().setUnderscoreName("test_query").build(); // Call the method under test MatchAllQueryBuilder matchAllQueryBuilder = MatchAllQueryBuilderProtoUtils.fromProto(matchAllQueryProto); @@ -55,7 +55,7 @@ public void testFromProtoWithName() { public void testFromProtoWithBoostAndName() { // Create a protobuf MatchAllQuery with boost and name - MatchAllQuery matchAllQueryProto = MatchAllQuery.newBuilder().setBoost(3.0f).setName("test_query").build(); + MatchAllQuery matchAllQueryProto = MatchAllQuery.newBuilder().setBoost(3.0f).setUnderscoreName("test_query").build(); // Call the method under test MatchAllQueryBuilder matchAllQueryBuilder = MatchAllQueryBuilderProtoUtils.fromProto(matchAllQueryProto); diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverterTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverterTests.java index 92ab02cc795f7..02fd7ebc0c101 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverterTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverterTests.java @@ -34,7 +34,7 @@ public void testGetHandledQueryCase() { public void testFromProto() { // Create a QueryContainer with MatchNoneQuery - MatchNoneQuery matchNoneQuery = MatchNoneQuery.newBuilder().setBoost(2.0f).setName("test_query").build(); + MatchNoneQuery matchNoneQuery = MatchNoneQuery.newBuilder().setBoost(2.0f).setUnderscoreName("test_query").build(); QueryContainer queryContainer = QueryContainer.newBuilder().setMatchNone(matchNoneQuery).build(); // Convert the query diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoUtilsTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoUtilsTests.java index 8149319241479..c22f9c15f8dc6 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoUtilsTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoUtilsTests.java @@ -42,7 +42,7 @@ public void testFromProtoWithBoost() { public void testFromProtoWithName() { // Create a protobuf MatchNoneQuery with name - MatchNoneQuery matchNoneQueryProto = MatchNoneQuery.newBuilder().setName("test_query").build(); + MatchNoneQuery matchNoneQueryProto = MatchNoneQuery.newBuilder().setUnderscoreName("test_query").build(); // Call the method under test MatchNoneQueryBuilder matchNoneQueryBuilder = MatchNoneQueryBuilderProtoUtils.fromProto(matchNoneQueryProto); @@ -55,7 +55,7 @@ public void testFromProtoWithName() { public void testFromProtoWithBoostAndName() { // Create a protobuf MatchNoneQuery with boost and name - MatchNoneQuery matchNoneQueryProto = MatchNoneQuery.newBuilder().setBoost(3.0f).setName("test_query").build(); + MatchNoneQuery matchNoneQueryProto = MatchNoneQuery.newBuilder().setBoost(3.0f).setUnderscoreName("test_query").build(); // Call the method under test MatchNoneQueryBuilder matchNoneQueryBuilder = MatchNoneQueryBuilderProtoUtils.fromProto(matchNoneQueryProto); diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverterTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverterTests.java index ef32bdb44e0f3..4c80449f030a8 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverterTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverterTests.java @@ -36,7 +36,7 @@ public void testFromProto() { .setField("test-field") .setValue(fieldValue) .setBoost(2.0f) - .setName("test_query") + .setUnderscoreName("test_query") .setCaseInsensitive(true) .build(); QueryContainer queryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtilsTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtilsTests.java index 583bdb920726e..03ae79aac89ec 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtilsTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtilsTests.java @@ -23,17 +23,14 @@ public class TermQueryBuilderProtoUtilsTests extends OpenSearchTestCase { public void testFromProtoWithStringValue() { // Create a protobuf TermQuery with string value TermQuery termQuery = TermQuery.newBuilder() - .setName("test_query") + .setField("test_field") + .setUnderscoreName("test_query") .setBoost(2.0f) .setValue(FieldValue.newBuilder().setStringValue("test_value").build()) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -46,17 +43,14 @@ public void testFromProtoWithStringValue() { public void testFromProtoWithNumberValue() { // Create a protobuf TermQuery with number value TermQuery termQuery = TermQuery.newBuilder() - .setName("test_query") + .setField("test_field") + .setUnderscoreName("test_query") .setBoost(2.0f) .setValue(FieldValue.newBuilder().setGeneralNumber(GeneralNumber.newBuilder().setFloatValue(10.5f).build()).build()) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -69,17 +63,14 @@ public void testFromProtoWithNumberValue() { public void testFromProtoWithBooleanValue() { // Create a protobuf TermQuery with boolean value TermQuery termQuery = TermQuery.newBuilder() - .setName("test_query") + .setField("test_field") + .setUnderscoreName("test_query") .setBoost(2.0f) .setValue(FieldValue.newBuilder().setBoolValue(true).build()) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -101,17 +92,14 @@ public void testFromProtoWithObjectMapValue() { } TermQuery termQuery = TermQuery.newBuilder() - .setName("test_query") + .setField("test_field") + .setUnderscoreName("test_query") .setBoost(2.0f) .setValue(FieldValue.newBuilder().setObjectMap(objectMapBuilder.build()).build()) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -128,14 +116,13 @@ public void testFromProtoWithObjectMapValue() { public void testFromProtoWithDefaultValues() { // Create a protobuf TermQuery with minimal values - TermQuery termQuery = TermQuery.newBuilder().setValue(FieldValue.newBuilder().setStringValue("test_value").build()).build(); - - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); + TermQuery termQuery = TermQuery.newBuilder() + .setField("test_field") + .setValue(FieldValue.newBuilder().setStringValue("test_value").build()) + .build(); // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -148,17 +135,14 @@ public void testFromProtoWithDefaultValues() { public void testFromProtoWithInvalidFieldValueType() { // Create a protobuf TermQuery with invalid field value type TermQuery termQuery = TermQuery.newBuilder() + .setField("test_field") .setValue(FieldValue.newBuilder().build()) // No value set .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test, should throw IllegalArgumentException IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> TermQueryBuilderProtoUtils.fromProto(termQueryProto) + () -> TermQueryBuilderProtoUtils.fromProto(termQuery) ); assertTrue( @@ -167,35 +151,17 @@ public void testFromProtoWithInvalidFieldValueType() { ); } - public void testFromProtoWithTooManyElements() { - // Create a map with too many elements - Map termQueryProto = new HashMap<>(); - termQueryProto.put("field1", TermQuery.newBuilder().build()); - termQueryProto.put("field2", TermQuery.newBuilder().build()); - - // Call the method under test, should throw IllegalArgumentException - IllegalArgumentException exception = expectThrows( - IllegalArgumentException.class, - () -> TermQueryBuilderProtoUtils.fromProto(termQueryProto) - ); - - assertTrue("Exception message should mention can only have 1 element", exception.getMessage().contains("can only have 1 element")); - } - public void testFromProtoWithInt32Value() { // Create a protobuf TermQuery with int32 value TermQuery termQuery = TermQuery.newBuilder() - .setName("test_query") + .setField("test_field") + .setUnderscoreName("test_query") .setBoost(2.0f) .setValue(FieldValue.newBuilder().setGeneralNumber(GeneralNumber.newBuilder().setInt32Value(42).build()).build()) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -208,19 +174,16 @@ public void testFromProtoWithInt32Value() { public void testFromProtoWithInt64Value() { // Create a protobuf TermQuery with int64 value TermQuery termQuery = TermQuery.newBuilder() - .setName("test_query") + .setField("test_field") + .setUnderscoreName("test_query") .setBoost(2.0f) .setValue( FieldValue.newBuilder().setGeneralNumber(GeneralNumber.newBuilder().setInt64Value(9223372036854775807L).build()).build() ) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -233,17 +196,14 @@ public void testFromProtoWithInt64Value() { public void testFromProtoWithDoubleValue() { // Create a protobuf TermQuery with double value TermQuery termQuery = TermQuery.newBuilder() - .setName("test_query") + .setField("test_field") + .setUnderscoreName("test_query") .setBoost(2.0f) .setValue(FieldValue.newBuilder().setGeneralNumber(GeneralNumber.newBuilder().setDoubleValue(3.14159).build()).build()) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -256,18 +216,15 @@ public void testFromProtoWithDoubleValue() { public void testFromProtoWithCaseInsensitive() { // Create a protobuf TermQuery with case insensitive flag TermQuery termQuery = TermQuery.newBuilder() - .setName("test_query") + .setField("test_field") + .setUnderscoreName("test_query") .setBoost(2.0f) .setValue(FieldValue.newBuilder().setStringValue("test_value").build()) .setCaseInsensitive(true) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test - TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQueryProto); + TermQueryBuilder termQueryBuilder = TermQueryBuilderProtoUtils.fromProto(termQuery); // Verify the result assertNotNull("TermQueryBuilder should not be null", termQueryBuilder); @@ -281,6 +238,7 @@ public void testFromProtoWithCaseInsensitive() { public void testFromProtoWithUnsupportedGeneralNumberType() { // Create a protobuf TermQuery with unsupported general number type TermQuery termQuery = TermQuery.newBuilder() + .setField("test_field") .setValue( FieldValue.newBuilder() .setGeneralNumber(GeneralNumber.newBuilder().build()) // No value set @@ -288,19 +246,15 @@ public void testFromProtoWithUnsupportedGeneralNumberType() { ) .build(); - // Create a map with field name and TermQuery - Map termQueryProto = new HashMap<>(); - termQueryProto.put("test_field", termQuery); - // Call the method under test, should throw IllegalArgumentException IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> TermQueryBuilderProtoUtils.fromProto(termQueryProto) + () -> TermQueryBuilderProtoUtils.fromProto(termQuery) ); assertTrue( "Exception message should mention unsupported general number type", - exception.getMessage().contains("Unsupported general nunber type") + exception.getMessage().contains("Unsupported general number type") ); } } diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.java index 99659c8ad28f2..1a22c4c09475a 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.java @@ -45,7 +45,7 @@ public void testFromProto() { TermsQueryField termsQueryField = TermsQueryField.newBuilder() .putAllTermsLookupFieldStringArrayMap(termsLookupFieldStringArrayMapMap) .setBoost(2.0f) - .setName("test_query") + .setUnderscoreName("test_query") .setValueType(ValueType.VALUE_TYPE_DEFAULT) .build(); QueryContainer queryContainer = QueryContainer.newBuilder().setTerms(termsQueryField).build(); diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java index e117d24a62188..7e0651baf879e 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java @@ -46,7 +46,7 @@ public void testFromProtoWithStringValues() { TermsQueryField termsQueryField = TermsQueryField.newBuilder() .putAllTermsLookupFieldStringArrayMap(termsLookupFieldStringArrayMapMap) .setBoost(2.0f) - .setName("test_query") + .setUnderscoreName("test_query") .build(); // Call the method under test @@ -86,7 +86,7 @@ public void testFromProtoWithTermsLookup() { TermsQueryField termsQueryField = TermsQueryField.newBuilder() .putAllTermsLookupFieldStringArrayMap(termsLookupFieldStringArrayMapMap) .setBoost(2.0f) - .setName("test_query") + .setUnderscoreName("test_query") .build(); // Call the method under test From b53b4469bb0eb8fbca01e15f067f8c874c08f4fe Mon Sep 17 00:00:00 2001 From: kkewwei Date: Fri, 1 Aug 2025 05:40:39 +0800 Subject: [PATCH 004/442] Optimize grouping for concurrent segment search by ensuring more even document distribution (#18451) Signed-off-by: kkewwei Signed-off-by: kkewwei --- CHANGELOG.md | 1 + .../internal/MaxTargetSliceSupplier.java | 31 ++++++++++-- .../internal/ContextIndexSearcherTests.java | 26 ++++------ .../internal/MaxTargetSliceSupplierTests.java | 50 +++++++++++++++++++ 4 files changed, 88 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb136d0121d53..2b553f795d1d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Make node duress values cacheable ([#18649](https://github.com/opensearch-project/OpenSearch/pull/18649)) - Change default value of remote_data_ratio, which is used in Searchable Snapshots and Writeable Warm from 0 to 5 and min allowed value to 1 ([#18767](https://github.com/opensearch-project/OpenSearch/pull/18767)) - Making multi rate limiters in repository dynamic [#18069](https://github.com/opensearch-project/OpenSearch/pull/18069) +- Optimize grouping for segment concurrent search by ensuring that documents within each group are as equal as possible ([#18451](https://github.com/opensearch-project/OpenSearch/pull/18451)) ### Dependencies - Bump `stefanzweifel/git-auto-commit-action` from 5 to 6 ([#18524](https://github.com/opensearch-project/OpenSearch/pull/18524)) diff --git a/server/src/main/java/org/opensearch/search/internal/MaxTargetSliceSupplier.java b/server/src/main/java/org/opensearch/search/internal/MaxTargetSliceSupplier.java index e2c61615601a8..51ebdc68ba099 100644 --- a/server/src/main/java/org/opensearch/search/internal/MaxTargetSliceSupplier.java +++ b/server/src/main/java/org/opensearch/search/internal/MaxTargetSliceSupplier.java @@ -15,6 +15,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.PriorityQueue; /** * Supplier to compute leaf slices based on passed in leaves and max target slice count to limit the number of computed slices. It sorts @@ -44,12 +45,34 @@ static IndexSearcher.LeafSlice[] getSlices(List leaves, int t for (int i = 0; i < targetSliceCount; ++i) { groupedLeaves.add(new ArrayList<>()); } - // distribute the slices in round-robin fashion - for (int idx = 0; idx < sortedLeaves.size(); ++idx) { - int currentGroup = idx % targetSliceCount; - groupedLeaves.get(currentGroup).add(IndexSearcher.LeafReaderContextPartition.createForEntireSegment(sortedLeaves.get(idx))); + + PriorityQueue groupQueue = new PriorityQueue<>(); + for (int i = 0; i < targetSliceCount; i++) { + groupQueue.offer(new Group(i)); + } + Group minGroup; + for (int i = 0; i < sortedLeaves.size(); ++i) { + minGroup = groupQueue.poll(); + groupedLeaves.get(minGroup.index).add(IndexSearcher.LeafReaderContextPartition.createForEntireSegment(sortedLeaves.get(i))); + minGroup.sum += sortedLeaves.get(i).reader().maxDoc(); + groupQueue.offer(minGroup); } return groupedLeaves.stream().map(IndexSearcher.LeafSlice::new).toArray(IndexSearcher.LeafSlice[]::new); } + + static class Group implements Comparable { + final int index; + int sum; + + public Group(int index) { + this.index = index; + this.sum = 0; + } + + @Override + public int compareTo(Group other) { + return Integer.compare(this.sum, other.sum); + } + } } diff --git a/server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java b/server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java index e645cd6a7723f..dd23318e61f7e 100644 --- a/server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java +++ b/server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java @@ -354,15 +354,12 @@ public void testSlicesInternal() throws Exception { expectedSliceCount = 4; slices = searcher.slicesInternal(leaves, expectedSliceCount); - // 4 slices will be created with 3 leaves in first 2 slices and 2 leaves in other slices + // 4 slices will be created with 3 leaves in first&last slices and 2 leaves in other slices assertEquals(expectedSliceCount, slices.length); - for (int i = 0; i < expectedSliceCount; ++i) { - if (i < 2) { - assertEquals(3, slices[i].partitions.length); - } else { - assertEquals(2, slices[i].partitions.length); - } - } + assertEquals(3, slices[0].partitions.length); + assertEquals(2, slices[1].partitions.length); + assertEquals(2, slices[2].partitions.length); + assertEquals(3, slices[3].partitions.length); } } } @@ -414,15 +411,12 @@ public void testGetSlicesWithNonNullExecutorButCSDisabled() throws Exception { int expectedSliceCount = 4; IndexSearcher.LeafSlice[] slices = searcher.slices(leaves); - // 4 slices will be created with 3 leaves in first 2 slices and 2 leaves in other slices + // 4 slices will be created with 3 leaves in first&last slices and 2 leaves in other slices assertEquals(expectedSliceCount, slices.length); - for (int i = 0; i < expectedSliceCount; ++i) { - if (i < 2) { - assertEquals(3, slices[i].partitions.length); - } else { - assertEquals(2, slices[i].partitions.length); - } - } + assertEquals(3, slices[0].partitions.length); + assertEquals(2, slices[1].partitions.length); + assertEquals(2, slices[2].partitions.length); + assertEquals(3, slices[3].partitions.length); } } } diff --git a/server/src/test/java/org/opensearch/search/internal/MaxTargetSliceSupplierTests.java b/server/src/test/java/org/opensearch/search/internal/MaxTargetSliceSupplierTests.java index f14670f187958..96a03540566c2 100644 --- a/server/src/test/java/org/opensearch/search/internal/MaxTargetSliceSupplierTests.java +++ b/server/src/test/java/org/opensearch/search/internal/MaxTargetSliceSupplierTests.java @@ -8,8 +8,17 @@ package org.opensearch.search.internal; +import org.apache.lucene.analysis.standard.StandardAnalyzer; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.StringField; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NoMergePolicy; import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.store.Directory; import org.opensearch.test.OpenSearchTestCase; import java.util.ArrayList; @@ -74,4 +83,45 @@ public void testEmptyLeaves() { IndexSearcher.LeafSlice[] slices = MaxTargetSliceSupplier.getSlices(new ArrayList<>(), 2); assertEquals(0, slices.length); } + + public void testOptimizedGroup() throws Exception { + try ( + final Directory directory = newDirectory(); + final IndexWriter iw = new IndexWriter( + directory, + new IndexWriterConfig(new StandardAnalyzer()).setMergePolicy(NoMergePolicy.INSTANCE) + ) + ) { + final String fieldValue = "value"; + for (int i = 0; i < 3; ++i) { + Document document = new Document(); + document.add(new StringField("field1", fieldValue, Field.Store.NO)); + iw.addDocument(document); + } + iw.commit(); + for (int i = 0; i < 1; ++i) { + Document document = new Document(); + document.add(new StringField("field1", fieldValue, Field.Store.NO)); + iw.addDocument(document); + } + iw.commit(); + for (int i = 0; i < 1; ++i) { + Document document = new Document(); + document.add(new StringField("field1", fieldValue, Field.Store.NO)); + iw.addDocument(document); + } + iw.commit(); + + try (DirectoryReader directoryReader = DirectoryReader.open(directory)) { + List leaves = directoryReader.leaves(); + assertEquals(3, leaves.size()); + IndexSearcher.LeafSlice[] slices = MaxTargetSliceSupplier.getSlices(leaves, 2); + assertEquals(1, slices[0].partitions.length); + assertEquals(3, slices[0].getMaxDocs()); + + assertEquals(2, slices[1].partitions.length); + assertEquals(2, slices[1].getMaxDocs()); + } + } + } } From e082c306330a71d53a36609b4f17c61551c43c26 Mon Sep 17 00:00:00 2001 From: Andre van de Ven <113951599+andrevandeven@users.noreply.github.com> Date: Thu, 31 Jul 2025 15:51:07 -0700 Subject: [PATCH 005/442] Add Fetch Phase to Search Profile (#18664) Signed-off-by: Andre van de Ven Signed-off-by: Andre van de Ven <113951599+andrevandeven@users.noreply.github.com> Co-authored-by: Andre van de Ven --- .idea/runConfigurations/Debug_OpenSearch.xml | 2 +- CHANGELOG.md | 1 + .../painless/130_script_fields_profile.yml | 69 ++ .../java/org/opensearch/search/CCSDuelIT.java | 6 +- .../test/search.profile/10_fetch_phase.yml | 193 ++++ .../search/profile/fetch/FetchProfilerIT.java | 390 ++++++++ .../search/profile/query/QueryProfilerIT.java | 2 - .../org/opensearch/search/SearchService.java | 6 + .../metrics/TopHitsAggregator.java | 2 +- .../opensearch/search/fetch/FetchPhase.java | 145 ++- .../fetch/subphase/InnerHitsContext.java | 6 + .../search/fetch/subphase/InnerHitsPhase.java | 4 +- .../fetch/subphase/ScriptFieldsPhase.java | 2 +- .../search/profile/ProfileMetricUtil.java | 9 + .../search/profile/ProfileShardResult.java | 31 + .../opensearch/search/profile/Profilers.java | 7 + .../profile/SearchProfileShardResults.java | 19 +- .../profile/fetch/FetchProfileBreakdown.java | 23 + .../fetch/FetchProfileShardResult.java | 78 ++ .../search/profile/fetch/FetchProfiler.java | 56 ++ .../search/profile/fetch/FetchTimingType.java | 36 + .../profile/fetch/FlatFetchProfileTree.java | 125 +++ .../search/profile/fetch/package-info.java | 11 + .../search/fetch/FetchPhaseTests.java | 54 ++ .../search/fetch/FetchProfilePhaseTests.java | 848 ++++++++++++++++++ .../SearchProfileShardResultsTests.java | 48 +- .../fetch/FetchProfileBreakdownTests.java | 45 + .../fetch/FetchProfileShardResultTests.java | 64 ++ .../profile/fetch/FetchProfilerTests.java | 99 ++ 29 files changed, 2345 insertions(+), 36 deletions(-) create mode 100644 modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/130_script_fields_profile.yml create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/search.profile/10_fetch_phase.yml create mode 100644 server/src/internalClusterTest/java/org/opensearch/search/profile/fetch/FetchProfilerIT.java create mode 100644 server/src/main/java/org/opensearch/search/profile/fetch/FetchProfileBreakdown.java create mode 100644 server/src/main/java/org/opensearch/search/profile/fetch/FetchProfileShardResult.java create mode 100644 server/src/main/java/org/opensearch/search/profile/fetch/FetchProfiler.java create mode 100644 server/src/main/java/org/opensearch/search/profile/fetch/FetchTimingType.java create mode 100644 server/src/main/java/org/opensearch/search/profile/fetch/FlatFetchProfileTree.java create mode 100644 server/src/main/java/org/opensearch/search/profile/fetch/package-info.java create mode 100644 server/src/test/java/org/opensearch/search/fetch/FetchProfilePhaseTests.java create mode 100644 server/src/test/java/org/opensearch/search/profile/fetch/FetchProfileBreakdownTests.java create mode 100644 server/src/test/java/org/opensearch/search/profile/fetch/FetchProfileShardResultTests.java create mode 100644 server/src/test/java/org/opensearch/search/profile/fetch/FetchProfilerTests.java diff --git a/.idea/runConfigurations/Debug_OpenSearch.xml b/.idea/runConfigurations/Debug_OpenSearch.xml index 0d8bf59823acf..2e167812615e1 100644 --- a/.idea/runConfigurations/Debug_OpenSearch.xml +++ b/.idea/runConfigurations/Debug_OpenSearch.xml @@ -8,4 +8,4 @@