From e3e348ff557bc2b53be5595e3efcc9907638e702 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 00:01:57 +0000 Subject: [PATCH 01/15] [GRPC] Extend transport-grpc plugin to support GRPC KNN queries and upgrade guava Signed-off-by: Karen Xu --- CHANGELOG.md | 1 + build.gradle | 6 +- .../query/KNNQueryBuilderProtoConverter.java | 32 ++ .../query/KNNQueryBuilderProtoUtils.java | 180 +++++++++++ ...st.search.query.QueryBuilderProtoConverter | 1 + .../KNNQueryBuilderProtoConverterTests.java | 76 +++++ .../query/KNNQueryBuilderProtoUtilsTests.java | 283 ++++++++++++++++++ 7 files changed, 577 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverter.java create mode 100644 src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtils.java create mode 100644 src/main/resources/META-INF/services/org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter create mode 100644 src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverterTests.java create mode 100644 src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtilsTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index c86150b210..48c319715d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) * Patch for supporting nested search in IndexBinaryHNSWCagra [#2824](https://github.com/opensearch-project/k-NN/pull/2824) * Support Asymmetric Distance Computation in Lucene-on-Faiss [#2781](https://github.com/opensearch-project/k-NN/pull/2781) +* Extend transport-grpc module to support GRPC KNN queries [#2792](https://github.com/opensearch-project/k-NN/pull/2792) ### Bug Fixes * [Remote Vector Index Build] Don't fall back to CPU on terminal failures [#2773](https://github.com/opensearch-project/k-NN/pull/2773) diff --git a/build.gradle b/build.gradle index 04f84a525e..0a91a98e2f 100644 --- a/build.gradle +++ b/build.gradle @@ -340,8 +340,10 @@ dependencies { api "org.opensearch:opensearch:${opensearch_version}" api project(":remote-index-build-client") compileOnly "org.opensearch.plugin:opensearch-scripting-painless-spi:${versions.opensearch}" - api group: 'com.google.guava', name: 'failureaccess', version:'1.0.1' - api group: 'com.google.guava', name: 'guava', version:'32.1.3-jre' + compileOnly "org.opensearch.plugin:transport-grpc:${opensearch_version}" + implementation "org.opensearch:protobufs:0.6.0" + api group: 'com.google.guava', name: 'failureaccess', version:'1.0.2' + api group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' api group: 'commons-lang', name: 'commons-lang', version: '2.6' testFixturesImplementation "org.opensearch.test:framework:${opensearch_version}" testImplementation group: 'net.bytebuddy', name: 'byte-buddy', version: "${versions.bytebuddy}" diff --git a/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverter.java b/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverter.java new file mode 100644 index 0000000000..8dc7c3e216 --- /dev/null +++ b/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverter.java @@ -0,0 +1,32 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.knn.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter; +import org.opensearch.protobufs.QueryContainer; + +/** + * Converter for KNN queries. + * This class implements the QueryBuilderProtoConverter interface to provide KNN query support + * for the gRPC transport plugin. + */ +public class KNNQueryBuilderProtoConverter implements QueryBuilderProtoConverter { + + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.KNN; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + if (queryContainer == null || queryContainer.getQueryContainerCase() != QueryContainer.QueryContainerCase.KNN) { + throw new IllegalArgumentException("QueryContainer does not contain a KNN query"); + } + + return KNNQueryBuilderProtoUtils.fromProto(queryContainer.getKnn()); + } +} diff --git a/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtils.java b/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtils.java new file mode 100644 index 0000000000..c8649f3c4f --- /dev/null +++ b/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtils.java @@ -0,0 +1,180 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.knn.grpc.proto.request.search.query; + +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.knn.index.query.KNNQueryBuilder; +import org.opensearch.knn.index.query.parser.KNNQueryBuilderParser; +import org.opensearch.knn.index.query.rescore.RescoreContext; +import org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverterRegistry; +import org.opensearch.protobufs.KnnQuery; +import org.opensearch.protobufs.KnnQueryRescore; +import org.opensearch.protobufs.QueryContainer; + +import java.util.List; + +/** + * Utility class for converting KNN Protocol Buffers to OpenSearch objects. + * This class provides methods to transform Protocol Buffer representations of KNN queries + * into their corresponding OpenSearch KNNQueryBuilder implementations for search operations. + */ +public class KNNQueryBuilderProtoUtils { + + // Registry for query conversion + private static QueryBuilderProtoConverterRegistry REGISTRY = new QueryBuilderProtoConverterRegistry(); + + private KNNQueryBuilderProtoUtils() { + // Utility class, no instances + } + + /** + * Sets the registry for testing purposes. + * + * @param registry The registry to use + */ + static void setRegistry(QueryBuilderProtoConverterRegistry registry) { + REGISTRY = registry; + } + + /** + * Gets the current registry. + * + * @return The current registry + */ + static QueryBuilderProtoConverterRegistry getRegistry() { + return REGISTRY; + } + + /** + * Converts a Protocol Buffer KnnQuery to an OpenSearch KNNQueryBuilder. + * Similar to {@link KNNQueryBuilderParser#fromXContent(XContentParser)}, this method + * parses the Protocol Buffer representation and creates a properly configured + * KNNQueryBuilder with the appropriate field name, vector, k, and other settings. + * + * @param knnQueryProto The Protocol Buffer KnnQuery to convert + * @return A configured KNNQueryBuilder instance + */ + public static QueryBuilder fromProto(KnnQuery knnQueryProto) { + + // Create a builder for the KNNQueryBuilder with the field name + KNNQueryBuilder.Builder builder = KNNQueryBuilder.builder(); + builder.fieldName(knnQueryProto.getField()); + + // Set vector + List vectorList = knnQueryProto.getVectorList(); + float[] vector = new float[vectorList.size()]; + for (int i = 0; i < vectorList.size(); i++) { + vector[i] = vectorList.get(i); + } + + builder.vector(vector); + + // Set k + builder.k(knnQueryProto.getK()); + + // Set filter if present + if (knnQueryProto.hasFilter()) { + // Convert the filter QueryContainer to a QueryBuilder + QueryContainer filterQueryContainer = knnQueryProto.getFilter(); + // Use our instance of the registry to convert the filter + builder.filter(REGISTRY.fromProto(filterQueryContainer)); + } + + // Set max distance if present + if (knnQueryProto.hasMaxDistance()) { + builder.maxDistance(knnQueryProto.getMaxDistance()); + } + + // Set min score if present + if (knnQueryProto.hasMinScore()) { + builder.minScore(knnQueryProto.getMinScore()); + } + + // Set method parameters if present + if (knnQueryProto.hasMethodParameters()) { + // Convert ObjectMap to Java Map + java.util.Map methodParameters = new java.util.HashMap<>(); + org.opensearch.protobufs.ObjectMap objectMap = knnQueryProto.getMethodParameters(); + + for (java.util.Map.Entry entry : objectMap.getFieldsMap().entrySet()) { + String key = entry.getKey(); + org.opensearch.protobufs.ObjectMap.Value value = entry.getValue(); + + // Extract the value based on its type + Object javaValue = null; + switch (value.getValueCase()) { + case INT32: + javaValue = value.getInt32(); + break; + case INT64: + javaValue = value.getInt64(); + break; + case FLOAT: + javaValue = value.getFloat(); + break; + case DOUBLE: + javaValue = value.getDouble(); + break; + case STRING: + javaValue = value.getString(); + break; + case BOOL: + javaValue = value.getBool(); + break; + // Handle other types as needed + default: + // Skip unsupported types + continue; + } + + methodParameters.put(key, javaValue); + } + + builder.methodParameters(methodParameters); + } + + // Set rescore if present + if (knnQueryProto.hasRescore()) { + // Convert the rescore KnnQueryRescore to a RescoreContext + KnnQueryRescore rescoreProto = knnQueryProto.getRescore(); + + if (rescoreProto.getKnnQueryRescoreCase() == KnnQueryRescore.KnnQueryRescoreCase.ENABLE) { + // If enable is true, use default rescore context, otherwise explicitly disable + if (rescoreProto.getEnable()) { + builder.rescoreContext(RescoreContext.getDefault()); + } else { + builder.rescoreContext(RescoreContext.EXPLICITLY_DISABLED_RESCORE_CONTEXT); + } + } else if (rescoreProto.getKnnQueryRescoreCase() == KnnQueryRescore.KnnQueryRescoreCase.CONTEXT) { + // If context is provided, create a RescoreContext with the specified oversample factor + org.opensearch.protobufs.RescoreContext contextProto = rescoreProto.getContext(); + if (contextProto.hasOversampleFactor()) { + builder.rescoreContext(RescoreContext.builder().oversampleFactor(contextProto.getOversampleFactor()).build()); + } else { + builder.rescoreContext(RescoreContext.getDefault()); + } + } + } + + // Set expandNested if present + if (knnQueryProto.hasExpandNestedDocs()) { + builder.expandNested(knnQueryProto.getExpandNestedDocs()); + } + + // Set boost if present + if (knnQueryProto.hasBoost()) { + builder.boost(knnQueryProto.getBoost()); + } + + // Set name if present + if (knnQueryProto.hasUnderscoreName()) { + builder.queryName(knnQueryProto.getUnderscoreName()); + } + + return builder.build(); + } +} diff --git a/src/main/resources/META-INF/services/org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter b/src/main/resources/META-INF/services/org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter new file mode 100644 index 0000000000..78475875cc --- /dev/null +++ b/src/main/resources/META-INF/services/org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter @@ -0,0 +1 @@ +org.opensearch.knn.grpc.proto.request.search.query.KNNQueryBuilderProtoConverter diff --git a/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverterTests.java b/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverterTests.java new file mode 100644 index 0000000000..cdcea4fa84 --- /dev/null +++ b/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverterTests.java @@ -0,0 +1,76 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.knn.grpc.proto.request.search.query; + +import org.junit.Before; +import org.junit.Test; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.knn.index.query.KNNQueryBuilder; +import org.opensearch.protobufs.KnnQuery; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.test.OpenSearchTestCase; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class KNNQueryBuilderProtoConverterTests extends OpenSearchTestCase { + + private KNNQueryBuilderProtoConverter converter; + private QueryContainer queryContainer; + private KnnQuery knnQuery; + + @Before + public void setup() { + converter = new KNNQueryBuilderProtoConverter(); + queryContainer = mock(QueryContainer.class); + knnQuery = mock(KnnQuery.class); + } + + @Test + public void testGetHandledQueryCase() { + assertEquals(QueryContainer.QueryContainerCase.KNN, converter.getHandledQueryCase()); + } + + @Test + public void testFromProto_validQuery() { + // Setup + when(queryContainer.getQueryContainerCase()).thenReturn(QueryContainer.QueryContainerCase.KNN); + when(queryContainer.getKnn()).thenReturn(knnQuery); + + // Mock the KNNQueryBuilderProtoUtils.fromProto method using PowerMock + KNNQueryBuilder expectedQueryBuilder = mock(KNNQueryBuilder.class); + + // Test + QueryBuilder result = converter.fromProto(queryContainer); + + // Verify + assertNotNull(result); + } + + @Test + public void testFromProto_nullQueryContainer() { + // Test + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(null)); + + // Verify + assertEquals("QueryContainer does not contain a KNN query", exception.getMessage()); + } + + @Test + public void testFromProto_wrongQueryContainerCase() { + // Setup + when(queryContainer.getQueryContainerCase()).thenReturn(QueryContainer.QueryContainerCase.BOOL); + + // Test + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> converter.fromProto(queryContainer) + ); + + // Verify + assertEquals("QueryContainer does not contain a KNN query", exception.getMessage()); + } +} diff --git a/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtilsTests.java b/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtilsTests.java new file mode 100644 index 0000000000..9ffe1439a8 --- /dev/null +++ b/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtilsTests.java @@ -0,0 +1,283 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.knn.grpc.proto.request.search.query; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.knn.index.query.KNNQueryBuilder; +import org.opensearch.knn.index.query.rescore.RescoreContext; +import org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter; +import org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverterRegistry; +import org.opensearch.protobufs.KnnQuery; +import org.opensearch.protobufs.KnnQueryRescore; +import org.opensearch.protobufs.ObjectMap; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Map; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +public class KNNQueryBuilderProtoUtilsTests extends OpenSearchTestCase { + + @Mock + private QueryBuilderProtoConverterRegistry mockRegistry; + + @Mock + private QueryBuilderProtoConverter mockConverter; + + @Mock + private QueryBuilder mockQueryBuilder; + + @Before + public void setup() { + MockitoAnnotations.openMocks(this); + } + + @Test + public void testFromProto_basicFields() { + KnnQuery knnQuery = KnnQuery.newBuilder().setField("test_field").addVector(1.0f).addVector(2.0f).addVector(3.0f).setK(5).build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertEquals("test_field", knnQueryBuilder.fieldName()); + assertArrayEquals(new float[] { 1.0f, 2.0f, 3.0f }, (float[]) knnQueryBuilder.vector(), 0.001f); + assertEquals(5, knnQueryBuilder.getK()); + } + + @Test + public void testFromProto_withBoost() { + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setBoost(2.5f) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertEquals(2.5f, knnQueryBuilder.boost(), 0.001f); + } + + @Test + public void testFromProto_withMaxDistance() { + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setMaxDistance(0.75f) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertEquals(0.75f, knnQueryBuilder.getMaxDistance(), 0.001f); + } + + @Test + public void testFromProto_withMinScore() { + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setMinScore(0.85f) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertEquals(0.85f, knnQueryBuilder.getMinScore(), 0.001f); + } + + @Test + public void testFromProto_withQueryName() { + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setUnderscoreName("test_query") + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertEquals("test_query", knnQueryBuilder.queryName()); + } + + @Test + public void testFromProto_withExpandNested() { + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setExpandNestedDocs(true) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertTrue(knnQueryBuilder.getExpandNested()); + } + + @Test + public void testFromProto_withMethodParameters() { + ObjectMap.Value intValue = ObjectMap.Value.newBuilder().setInt32(100).build(); + ObjectMap.Value floatValue = ObjectMap.Value.newBuilder().setFloat(0.5f).build(); + ObjectMap methodParams = ObjectMap.newBuilder().putFields("ef_search", intValue).putFields("nprobes", floatValue).build(); + + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setMethodParameters(methodParams) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + Map methodParameters = knnQueryBuilder.getMethodParameters(); + assertNotNull(methodParameters); + assertEquals(2, methodParameters.size()); + assertEquals(100, methodParameters.get("ef_search")); + assertEquals(0.5f, methodParameters.get("nprobes")); + } + + @Test + public void testFromProto_withFilter() { + QueryContainer filterContainer = QueryContainer.newBuilder().build(); + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setFilter(filterContainer) + .build(); + + QueryBuilderProtoConverterRegistry originalRegistry = KNNQueryBuilderProtoUtils.getRegistry(); + + try { + KNNQueryBuilderProtoUtils.setRegistry(mockRegistry); + when(mockRegistry.fromProto(any())).thenReturn(mockQueryBuilder); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertNotNull(knnQueryBuilder.getFilter()); + assertEquals(mockQueryBuilder, knnQueryBuilder.getFilter()); + } finally { + KNNQueryBuilderProtoUtils.setRegistry(originalRegistry); + } + } + + @Test + public void testFromProto_withRescoreEnable() { + KnnQueryRescore rescore = KnnQueryRescore.newBuilder().setEnable(true).build(); + + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setRescore(rescore) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertNotNull(knnQueryBuilder.getRescoreContext()); + assertEquals(RescoreContext.getDefault(), knnQueryBuilder.getRescoreContext()); + } + + @Test + public void testFromProto_withRescoreDisable() { + KnnQueryRescore rescore = KnnQueryRescore.newBuilder().setEnable(false).build(); + + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setRescore(rescore) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertNotNull(knnQueryBuilder.getRescoreContext()); + assertEquals(RescoreContext.EXPLICITLY_DISABLED_RESCORE_CONTEXT, knnQueryBuilder.getRescoreContext()); + } + + @Test + public void testFromProto_withRescoreContext() { + org.opensearch.protobufs.RescoreContext rescoreContext = org.opensearch.protobufs.RescoreContext.newBuilder() + .setOversampleFactor(3.5f) + .build(); + + KnnQueryRescore rescore = KnnQueryRescore.newBuilder().setContext(rescoreContext).build(); + + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setRescore(rescore) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertNotNull(knnQueryBuilder.getRescoreContext()); + assertEquals(3.5f, knnQueryBuilder.getRescoreContext().getOversampleFactor(), 0.001f); + } + + @Test + public void testFromProto_withRescoreContextNoOversampleFactor() { + org.opensearch.protobufs.RescoreContext rescoreContext = org.opensearch.protobufs.RescoreContext.newBuilder().build(); + + KnnQueryRescore rescore = KnnQueryRescore.newBuilder().setContext(rescoreContext).build(); + + KnnQuery knnQuery = KnnQuery.newBuilder() + .setField("test_field") + .addVector(1.0f) + .addVector(2.0f) + .addVector(3.0f) + .setK(5) + .setRescore(rescore) + .build(); + + QueryBuilder result = KNNQueryBuilderProtoUtils.fromProto(knnQuery); + assertTrue(result instanceof KNNQueryBuilder); + KNNQueryBuilder knnQueryBuilder = (KNNQueryBuilder) result; + assertNotNull(knnQueryBuilder.getRescoreContext()); + assertEquals(RescoreContext.getDefault(), knnQueryBuilder.getRescoreContext()); + } +} From 89623295d3acc23edaea26a61eb434b2184bf47b Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 02:25:58 +0000 Subject: [PATCH 02/15] use @utility and refactor into smaller methods Signed-off-by: Karen Xu --- .../query/KNNQueryBuilderProtoUtils.java | 252 +++++++++++------- .../query/KNNQueryBuilderProtoUtilsTests.java | 10 +- 2 files changed, 153 insertions(+), 109 deletions(-) diff --git a/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtils.java b/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtils.java index c8649f3c4f..46738122a7 100644 --- a/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtils.java +++ b/src/main/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtils.java @@ -5,38 +5,39 @@ package org.opensearch.knn.grpc.proto.request.search.query; +import lombok.experimental.UtilityClass; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.QueryBuilder; import org.opensearch.knn.index.query.KNNQueryBuilder; import org.opensearch.knn.index.query.parser.KNNQueryBuilderParser; +import org.opensearch.knn.index.query.request.MethodParameter; import org.opensearch.knn.index.query.rescore.RescoreContext; import org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverterRegistry; import org.opensearch.protobufs.KnnQuery; import org.opensearch.protobufs.KnnQueryRescore; import org.opensearch.protobufs.QueryContainer; +import java.util.HashMap; import java.util.List; +import java.util.Map; /** * Utility class for converting KNN Protocol Buffers to OpenSearch objects. * This class provides methods to transform Protocol Buffer representations of KNN queries * into their corresponding OpenSearch KNNQueryBuilder implementations for search operations. */ +@UtilityClass public class KNNQueryBuilderProtoUtils { // Registry for query conversion private static QueryBuilderProtoConverterRegistry REGISTRY = new QueryBuilderProtoConverterRegistry(); - private KNNQueryBuilderProtoUtils() { - // Utility class, no instances - } - /** * Sets the registry for testing purposes. * * @param registry The registry to use */ - static void setRegistry(QueryBuilderProtoConverterRegistry registry) { + void setRegistry(QueryBuilderProtoConverterRegistry registry) { REGISTRY = registry; } @@ -45,136 +46,181 @@ static void setRegistry(QueryBuilderProtoConverterRegistry registry) { * * @return The current registry */ - static QueryBuilderProtoConverterRegistry getRegistry() { + QueryBuilderProtoConverterRegistry getRegistry() { return REGISTRY; } /** - * Converts a Protocol Buffer KnnQuery to an OpenSearch KNNQueryBuilder. - * Similar to {@link KNNQueryBuilderParser#fromXContent(XContentParser)}, this method - * parses the Protocol Buffer representation and creates a properly configured - * KNNQueryBuilder with the appropriate field name, vector, k, and other settings. - * - * @param knnQueryProto The Protocol Buffer KnnQuery to convert - * @return A configured KNNQueryBuilder instance - */ - public static QueryBuilder fromProto(KnnQuery knnQueryProto) { - - // Create a builder for the KNNQueryBuilder with the field name + * Converts a Protocol Buffer KnnQuery to an OpenSearch KNNQueryBuilder. + * This method follows the exact same pattern as {@link KNNQueryBuilderParser#fromXContent(XContentParser)} + * to ensure parsing consistency and compatibility. + * + * @param knnQueryProto The Protocol Buffer KnnQuery to convert + * @return A configured KNNQueryBuilder instance + */ + public QueryBuilder fromProto(KnnQuery knnQueryProto) { + // Create builder using the internal parser pattern like XContent parsing KNNQueryBuilder.Builder builder = KNNQueryBuilder.builder(); - builder.fieldName(knnQueryProto.getField()); - // Set vector - List vectorList = knnQueryProto.getVectorList(); - float[] vector = new float[vectorList.size()]; - for (int i = 0; i < vectorList.size(); i++) { - vector[i] = vectorList.get(i); - } - - builder.vector(vector); + // Set field name (equivalent to fieldName parsing in XContent) + builder.fieldName(knnQueryProto.getField()); - // Set k - builder.k(knnQueryProto.getK()); + // Set vector (equivalent to VECTOR_FIELD parsing) + builder.vector(convertVector(knnQueryProto.getVectorList())); - // Set filter if present - if (knnQueryProto.hasFilter()) { - // Convert the filter QueryContainer to a QueryBuilder - QueryContainer filterQueryContainer = knnQueryProto.getFilter(); - // Use our instance of the registry to convert the filter - builder.filter(REGISTRY.fromProto(filterQueryContainer)); + // Set k if present (equivalent to K_FIELD parsing) + if (knnQueryProto.getK() > 0) { + builder.k(knnQueryProto.getK()); } - // Set max distance if present - if (knnQueryProto.hasMaxDistance()) { + // Set maxDistance if present (equivalent to MAX_DISTANCE_FIELD parsing) + else if (knnQueryProto.hasMaxDistance()) { builder.maxDistance(knnQueryProto.getMaxDistance()); } - // Set min score if present - if (knnQueryProto.hasMinScore()) { + // Set minScore if present (equivalent to MIN_SCORE_FIELD parsing) + else if (knnQueryProto.hasMinScore()) { builder.minScore(knnQueryProto.getMinScore()); } - // Set method parameters if present + // Set method parameters (equivalent to METHOD_PARAMS_FIELD parsing) if (knnQueryProto.hasMethodParameters()) { - // Convert ObjectMap to Java Map - java.util.Map methodParameters = new java.util.HashMap<>(); - org.opensearch.protobufs.ObjectMap objectMap = knnQueryProto.getMethodParameters(); - - for (java.util.Map.Entry entry : objectMap.getFieldsMap().entrySet()) { - String key = entry.getKey(); - org.opensearch.protobufs.ObjectMap.Value value = entry.getValue(); - - // Extract the value based on its type - Object javaValue = null; - switch (value.getValueCase()) { - case INT32: - javaValue = value.getInt32(); - break; - case INT64: - javaValue = value.getInt64(); - break; - case FLOAT: - javaValue = value.getFloat(); - break; - case DOUBLE: - javaValue = value.getDouble(); - break; - case STRING: - javaValue = value.getString(); - break; - case BOOL: - javaValue = value.getBool(); - break; - // Handle other types as needed - default: - // Skip unsupported types - continue; - } - - methodParameters.put(key, javaValue); - } - + Map methodParameters = convertMethodParameters(knnQueryProto.getMethodParameters()); builder.methodParameters(methodParameters); } - // Set rescore if present - if (knnQueryProto.hasRescore()) { - // Convert the rescore KnnQueryRescore to a RescoreContext - KnnQueryRescore rescoreProto = knnQueryProto.getRescore(); - - if (rescoreProto.getKnnQueryRescoreCase() == KnnQueryRescore.KnnQueryRescoreCase.ENABLE) { - // If enable is true, use default rescore context, otherwise explicitly disable - if (rescoreProto.getEnable()) { - builder.rescoreContext(RescoreContext.getDefault()); - } else { - builder.rescoreContext(RescoreContext.EXPLICITLY_DISABLED_RESCORE_CONTEXT); - } - } else if (rescoreProto.getKnnQueryRescoreCase() == KnnQueryRescore.KnnQueryRescoreCase.CONTEXT) { - // If context is provided, create a RescoreContext with the specified oversample factor - org.opensearch.protobufs.RescoreContext contextProto = rescoreProto.getContext(); - if (contextProto.hasOversampleFactor()) { - builder.rescoreContext(RescoreContext.builder().oversampleFactor(contextProto.getOversampleFactor()).build()); - } else { - builder.rescoreContext(RescoreContext.getDefault()); - } - } + // Set filter (equivalent to FILTER_FIELD parsing) + if (knnQueryProto.hasFilter()) { + QueryContainer filterQueryContainer = knnQueryProto.getFilter(); + builder.filter(REGISTRY.fromProto(filterQueryContainer)); } - // Set expandNested if present - if (knnQueryProto.hasExpandNestedDocs()) { - builder.expandNested(knnQueryProto.getExpandNestedDocs()); + // Set rescore (equivalent to RESCORE_FIELD parsing) + if (knnQueryProto.hasRescore()) { + RescoreContext rescoreContext = convertRescoreContext(knnQueryProto.getRescore()); + builder.rescoreContext(rescoreContext); } - // Set boost if present + // Set boost (equivalent to BOOST_FIELD parsing) if (knnQueryProto.hasBoost()) { builder.boost(knnQueryProto.getBoost()); } - // Set name if present + // Set query name (equivalent to NAME_FIELD parsing) if (knnQueryProto.hasUnderscoreName()) { builder.queryName(knnQueryProto.getUnderscoreName()); } + // Set expandNested (equivalent to EXPAND_NESTED_FIELD parsing) + if (knnQueryProto.hasExpandNestedDocs()) { + builder.expandNested(knnQueryProto.getExpandNestedDocs()); + } + return builder.build(); } + + /** + * Converts a Protocol Buffer vector list to a float array. + * + * @param vectorList The Protocol Buffer vector list + * @return The converted float array + */ + private float[] convertVector(List vectorList) { + float[] vector = new float[vectorList.size()]; + for (int i = 0; i < vectorList.size(); i++) { + vector[i] = vectorList.get(i); + } + return vector; + } + + /** + * Converts Protocol Buffer method parameters following the exact same pattern as + * {@link MethodParametersParser#fromXContent(XContentParser)} to ensure consistency. + * + * @param objectMap The Protocol Buffer ObjectMap to convert + * @return The converted method parameters Map + */ + private Map convertMethodParameters(org.opensearch.protobufs.ObjectMap objectMap) { + // First convert Protocol Buffer to raw Map (equivalent to parser.map()) + Map rawMethodParameters = new HashMap<>(); + for (Map.Entry entry : objectMap.getFieldsMap().entrySet()) { + String key = entry.getKey(); + Object value = convertObjectMapValue(entry.getValue()); + if (value != null) { + rawMethodParameters.put(key, value); + } + } + + // Then process through MethodParameter.parse() exactly like XContent parsing does + Map processedMethodParameters = new HashMap<>(); + for (Map.Entry entry : rawMethodParameters.entrySet()) { + String name = entry.getKey(); + Object value = entry.getValue(); + + // Find the MethodParameter enum (same as XContent parsing) + MethodParameter parameter = MethodParameter.enumOf(name); + if (parameter == null) { + throw new IllegalArgumentException("unknown method parameter found [" + name + "]"); + } + + try { + // Parse using MethodParameter.parse() - this handles type conversion properly + Object parsedValue = parameter.parse(value); + processedMethodParameters.put(name, parsedValue); + } catch (Exception exception) { + throw new IllegalArgumentException("Error parsing method parameter [" + name + "]: " + exception.getMessage()); + } + } + + return processedMethodParameters.isEmpty() ? null : processedMethodParameters; + } + + /** + * Converts a Protocol Buffer ObjectMap.Value to a Java Object. + * + * @param value The Protocol Buffer Value to convert + * @return The converted Java Object, or null if unsupported type + */ + private Object convertObjectMapValue(org.opensearch.protobufs.ObjectMap.Value value) { + switch (value.getValueCase()) { + case INT32: + return value.getInt32(); + case INT64: + return value.getInt64(); + case FLOAT: + return value.getFloat(); + case DOUBLE: + return value.getDouble(); + case STRING: + return value.getString(); + case BOOL: + return value.getBool(); + default: + // Skip unsupported types + return null; + } + } + + /** + * Converts a Protocol Buffer KnnQueryRescore to a RescoreContext. + * + * @param rescoreProto The Protocol Buffer KnnQueryRescore to convert + * @return The converted RescoreContext + */ + private RescoreContext convertRescoreContext(KnnQueryRescore rescoreProto) { + switch (rescoreProto.getKnnQueryRescoreCase()) { + case ENABLE: + return rescoreProto.getEnable() ? RescoreContext.getDefault() : RescoreContext.EXPLICITLY_DISABLED_RESCORE_CONTEXT; + + case CONTEXT: + org.opensearch.protobufs.RescoreContext contextProto = rescoreProto.getContext(); + return contextProto.hasOversampleFactor() + ? RescoreContext.builder().oversampleFactor(contextProto.getOversampleFactor()).build() + : RescoreContext.getDefault(); + + default: + return RescoreContext.getDefault(); + } + } + } diff --git a/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtilsTests.java b/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtilsTests.java index 9ffe1439a8..e82fac5b5a 100644 --- a/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtilsTests.java +++ b/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoUtilsTests.java @@ -79,7 +79,6 @@ public void testFromProto_withMaxDistance() { .addVector(1.0f) .addVector(2.0f) .addVector(3.0f) - .setK(5) .setMaxDistance(0.75f) .build(); @@ -97,7 +96,6 @@ public void testFromProto_withMinScore() { .addVector(1.0f) .addVector(2.0f) .addVector(3.0f) - .setK(5) .setMinScore(0.85f) .build(); @@ -143,9 +141,9 @@ public void testFromProto_withExpandNested() { @Test public void testFromProto_withMethodParameters() { - ObjectMap.Value intValue = ObjectMap.Value.newBuilder().setInt32(100).build(); - ObjectMap.Value floatValue = ObjectMap.Value.newBuilder().setFloat(0.5f).build(); - ObjectMap methodParams = ObjectMap.newBuilder().putFields("ef_search", intValue).putFields("nprobes", floatValue).build(); + ObjectMap.Value efSearchValue = ObjectMap.Value.newBuilder().setInt32(100).build(); + ObjectMap.Value nprobesValue = ObjectMap.Value.newBuilder().setInt32(10).build(); + ObjectMap methodParams = ObjectMap.newBuilder().putFields("ef_search", efSearchValue).putFields("nprobes", nprobesValue).build(); KnnQuery knnQuery = KnnQuery.newBuilder() .setField("test_field") @@ -163,7 +161,7 @@ public void testFromProto_withMethodParameters() { assertNotNull(methodParameters); assertEquals(2, methodParameters.size()); assertEquals(100, methodParameters.get("ef_search")); - assertEquals(0.5f, methodParameters.get("nprobes")); + assertEquals(10, methodParameters.get("nprobes")); } @Test From 98a17ed947acdb69e316a28d4862a867e244d6ed Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 16:37:58 +0000 Subject: [PATCH 03/15] Force bundle guava dependencies Signed-off-by: Karen Xu --- build.gradle | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 0a91a98e2f..f52f655034 100644 --- a/build.gradle +++ b/build.gradle @@ -320,6 +320,14 @@ opensearchplugin { noticeFile = rootProject.file('NOTICE.txt') } +// Force bundle Guava dependencies +tasks.named("bundlePlugin").configure { + from(configurations.runtimeClasspath) { + include "guava-*.jar" + include "failureaccess-*.jar" + } +} + tasks.named("integTest").configure { it.dependsOn(project.tasks.named("bundlePlugin")) } @@ -341,7 +349,7 @@ dependencies { api project(":remote-index-build-client") compileOnly "org.opensearch.plugin:opensearch-scripting-painless-spi:${versions.opensearch}" compileOnly "org.opensearch.plugin:transport-grpc:${opensearch_version}" - implementation "org.opensearch:protobufs:0.6.0" + compileOnly "org.opensearch:protobufs:0.6.0" api group: 'com.google.guava', name: 'failureaccess', version:'1.0.2' api group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' api group: 'commons-lang', name: 'commons-lang', version: '2.6' From 017a779fc15c9b3b09996d0476f0b0e5d003a4ea Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 17:07:57 +0000 Subject: [PATCH 04/15] Force bundle failure access only not guava, and upgrade guava to match security plugin version Signed-off-by: Karen Xu --- build.gradle | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/build.gradle b/build.gradle index f52f655034..0601d3180e 100644 --- a/build.gradle +++ b/build.gradle @@ -320,10 +320,9 @@ opensearchplugin { noticeFile = rootProject.file('NOTICE.txt') } -// Force bundle Guava dependencies +// Force bundle failureaccess dependency - required because compileOnly transport-grpc dependency tasks.named("bundlePlugin").configure { from(configurations.runtimeClasspath) { - include "guava-*.jar" include "failureaccess-*.jar" } } @@ -350,8 +349,8 @@ dependencies { compileOnly "org.opensearch.plugin:opensearch-scripting-painless-spi:${versions.opensearch}" compileOnly "org.opensearch.plugin:transport-grpc:${opensearch_version}" compileOnly "org.opensearch:protobufs:0.6.0" - api group: 'com.google.guava', name: 'failureaccess', version:'1.0.2' - api group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' + api group: 'com.google.guava', name: 'failureaccess', version:'1.0.1' // Keep same version as main branch + api group: 'com.google.guava', name: 'guava', version:'33.4.8-jre' api group: 'commons-lang', name: 'commons-lang', version: '2.6' testFixturesImplementation "org.opensearch.test:framework:${opensearch_version}" testImplementation group: 'net.bytebuddy', name: 'byte-buddy', version: "${versions.bytebuddy}" From 5d92f7ee72bb8d0fb0c520acaa709eb229593bba Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 17:11:14 +0000 Subject: [PATCH 05/15] Downgrade failure access back and dont force Signed-off-by: Karen Xu --- build.gradle | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/build.gradle b/build.gradle index 0601d3180e..bd47347eb1 100644 --- a/build.gradle +++ b/build.gradle @@ -320,12 +320,6 @@ opensearchplugin { noticeFile = rootProject.file('NOTICE.txt') } -// Force bundle failureaccess dependency - required because compileOnly transport-grpc dependency -tasks.named("bundlePlugin").configure { - from(configurations.runtimeClasspath) { - include "failureaccess-*.jar" - } -} tasks.named("integTest").configure { it.dependsOn(project.tasks.named("bundlePlugin")) @@ -349,7 +343,7 @@ dependencies { compileOnly "org.opensearch.plugin:opensearch-scripting-painless-spi:${versions.opensearch}" compileOnly "org.opensearch.plugin:transport-grpc:${opensearch_version}" compileOnly "org.opensearch:protobufs:0.6.0" - api group: 'com.google.guava', name: 'failureaccess', version:'1.0.1' // Keep same version as main branch + api group: 'com.google.guava', name: 'failureaccess', version:'1.0.1' api group: 'com.google.guava', name: 'guava', version:'33.4.8-jre' api group: 'commons-lang', name: 'commons-lang', version: '2.6' testFixturesImplementation "org.opensearch.test:framework:${opensearch_version}" From 59f743026eafaac00d1c50f5abaa789fc7f423a3 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 17:20:27 +0000 Subject: [PATCH 06/15] force deps for tests Signed-off-by: Karen Xu --- build.gradle | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index bd47347eb1..114de395c7 100644 --- a/build.gradle +++ b/build.gradle @@ -320,7 +320,6 @@ opensearchplugin { noticeFile = rootProject.file('NOTICE.txt') } - tasks.named("integTest").configure { it.dependsOn(project.tasks.named("bundlePlugin")) } @@ -337,6 +336,14 @@ task release(type: Copy, group: 'build') { //****************************************************************************/ // Dependencies //****************************************************************************/ +// Force resolution of conflicting dependencies for tests +configurations.matching { it.name.contains('test') || it.name.contains('Test') }.all { + resolutionStrategy { + force 'com.google.guava:guava:33.4.8-jre' + force 'com.google.guava:failureaccess:1.0.1' + } +} + dependencies { api "org.opensearch:opensearch:${opensearch_version}" api project(":remote-index-build-client") From b5bd43278109ec23631f1af26904419850615554 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 18:12:40 +0000 Subject: [PATCH 07/15] use 32.1.3 original Signed-off-by: Karen Xu --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 114de395c7..1aa96ed7c1 100644 --- a/build.gradle +++ b/build.gradle @@ -339,7 +339,7 @@ task release(type: Copy, group: 'build') { // Force resolution of conflicting dependencies for tests configurations.matching { it.name.contains('test') || it.name.contains('Test') }.all { resolutionStrategy { - force 'com.google.guava:guava:33.4.8-jre' + force 'com.google.guava:guava:32.1.3-jre' force 'com.google.guava:failureaccess:1.0.1' } } @@ -351,7 +351,7 @@ dependencies { compileOnly "org.opensearch.plugin:transport-grpc:${opensearch_version}" compileOnly "org.opensearch:protobufs:0.6.0" api group: 'com.google.guava', name: 'failureaccess', version:'1.0.1' - api group: 'com.google.guava', name: 'guava', version:'33.4.8-jre' + api group: 'com.google.guava', name: 'guava', version:'32.1.3-jre' api group: 'commons-lang', name: 'commons-lang', version: '2.6' testFixturesImplementation "org.opensearch.test:framework:${opensearch_version}" testImplementation group: 'net.bytebuddy', name: 'byte-buddy', version: "${versions.bytebuddy}" From e996916b555420ef2f758036b95a38d8a0302803 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 18:32:14 +0000 Subject: [PATCH 08/15] move from changlog to 3.2. release notes Signed-off-by: Karen Xu --- CHANGELOG.md | 3 +-- release-notes/opensearch-k-NN.release-notes-3.2.0.0.md | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1279f91f14..7fa3dcff77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,11 +20,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) * Patch for supporting nested search in IndexBinaryHNSWCagra [#2824](https://github.com/opensearch-project/k-NN/pull/2824) * Support Asymmetric Distance Computation in Lucene-on-Faiss [#2781](https://github.com/opensearch-project/k-NN/pull/2781) -* Extend transport-grpc module to support GRPC KNN queries [#2792](https://github.com/opensearch-project/k-NN/pull/2792) ### Bug Fixes * [Remote Vector Index Build] Don't fall back to CPU on terminal failures [#2773](https://github.com/opensearch-project/k-NN/pull/2773) * Fix @ collision in NativeMemoryCacheKeyHelper for vector index filenames containing @ characters [#2810](https://github.com/opensearch-project/k-NN/pull/2810) -## [Unreleased 3.3](https://github.com/opensearch-project/k-NN/compare/main...HEAD) \ No newline at end of file +## [Unreleased 3.3](https://github.com/opensearch-project/k-NN/compare/main...HEAD) diff --git a/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md b/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md index 2375499a87..5c258fb412 100644 --- a/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md +++ b/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md @@ -12,6 +12,7 @@ Compatible with OpenSearch 3.2.0 * Support GPU indexing for FP16, Byte and Binary [#2819](https://github.com/opensearch-project/k-NN/pull/2819) * Add random rotation feature to binary encoder for improving recall on certain datasets [#2718](https://github.com/opensearch-project/k-NN/pull/2718) * Asymmetric Distance Computation (ADC) for binary quantized faiss indices [#2733](https://github.com/opensearch-project/k-NN/pull/2733) +* Extend transport-grpc module to support GRPC KNN queries [#2792](https://github.com/opensearch-project/k-NN/pull/2792) ### Enhancements * Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) From 590361cf99957f504d2d79dfaf28a95e604f0251 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 18:33:56 +0000 Subject: [PATCH 09/15] Revert "move from changlog to 3.2. release notes" This reverts commit e996916b555420ef2f758036b95a38d8a0302803. Signed-off-by: Karen Xu --- CHANGELOG.md | 3 ++- release-notes/opensearch-k-NN.release-notes-3.2.0.0.md | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fa3dcff77..1279f91f14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,10 +20,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) * Patch for supporting nested search in IndexBinaryHNSWCagra [#2824](https://github.com/opensearch-project/k-NN/pull/2824) * Support Asymmetric Distance Computation in Lucene-on-Faiss [#2781](https://github.com/opensearch-project/k-NN/pull/2781) +* Extend transport-grpc module to support GRPC KNN queries [#2792](https://github.com/opensearch-project/k-NN/pull/2792) ### Bug Fixes * [Remote Vector Index Build] Don't fall back to CPU on terminal failures [#2773](https://github.com/opensearch-project/k-NN/pull/2773) * Fix @ collision in NativeMemoryCacheKeyHelper for vector index filenames containing @ characters [#2810](https://github.com/opensearch-project/k-NN/pull/2810) -## [Unreleased 3.3](https://github.com/opensearch-project/k-NN/compare/main...HEAD) +## [Unreleased 3.3](https://github.com/opensearch-project/k-NN/compare/main...HEAD) \ No newline at end of file diff --git a/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md b/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md index 5c258fb412..2375499a87 100644 --- a/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md +++ b/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md @@ -12,7 +12,6 @@ Compatible with OpenSearch 3.2.0 * Support GPU indexing for FP16, Byte and Binary [#2819](https://github.com/opensearch-project/k-NN/pull/2819) * Add random rotation feature to binary encoder for improving recall on certain datasets [#2718](https://github.com/opensearch-project/k-NN/pull/2718) * Asymmetric Distance Computation (ADC) for binary quantized faiss indices [#2733](https://github.com/opensearch-project/k-NN/pull/2733) -* Extend transport-grpc module to support GRPC KNN queries [#2792](https://github.com/opensearch-project/k-NN/pull/2792) ### Enhancements * Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) From d11c5d716a71ad403ec605e12b11d3896ab69705 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 18:40:57 +0000 Subject: [PATCH 10/15] Revert "Revert "move from changlog to 3.2. release notes"" This reverts commit 590361cf99957f504d2d79dfaf28a95e604f0251. Signed-off-by: Karen Xu --- CHANGELOG.md | 3 +-- release-notes/opensearch-k-NN.release-notes-3.2.0.0.md | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1279f91f14..7fa3dcff77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,11 +20,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) * Patch for supporting nested search in IndexBinaryHNSWCagra [#2824](https://github.com/opensearch-project/k-NN/pull/2824) * Support Asymmetric Distance Computation in Lucene-on-Faiss [#2781](https://github.com/opensearch-project/k-NN/pull/2781) -* Extend transport-grpc module to support GRPC KNN queries [#2792](https://github.com/opensearch-project/k-NN/pull/2792) ### Bug Fixes * [Remote Vector Index Build] Don't fall back to CPU on terminal failures [#2773](https://github.com/opensearch-project/k-NN/pull/2773) * Fix @ collision in NativeMemoryCacheKeyHelper for vector index filenames containing @ characters [#2810](https://github.com/opensearch-project/k-NN/pull/2810) -## [Unreleased 3.3](https://github.com/opensearch-project/k-NN/compare/main...HEAD) \ No newline at end of file +## [Unreleased 3.3](https://github.com/opensearch-project/k-NN/compare/main...HEAD) diff --git a/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md b/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md index 2375499a87..5c258fb412 100644 --- a/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md +++ b/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md @@ -12,6 +12,7 @@ Compatible with OpenSearch 3.2.0 * Support GPU indexing for FP16, Byte and Binary [#2819](https://github.com/opensearch-project/k-NN/pull/2819) * Add random rotation feature to binary encoder for improving recall on certain datasets [#2718](https://github.com/opensearch-project/k-NN/pull/2718) * Asymmetric Distance Computation (ADC) for binary quantized faiss indices [#2733](https://github.com/opensearch-project/k-NN/pull/2733) +* Extend transport-grpc module to support GRPC KNN queries [#2792](https://github.com/opensearch-project/k-NN/pull/2792) ### Enhancements * Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) From fbb43eb8c0a6f78d1a2d83eedda802c84458691a Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 19:13:05 +0000 Subject: [PATCH 11/15] force resolution for all, not just tests, and resolve changelog conflicts Signed-off-by: Karen Xu --- CHANGELOG.md | 24 +----------------------- build.gradle | 5 ++--- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fa3dcff77..dd3f6c505f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,26 +4,4 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). See the [CONTRIBUTING guide](./CONTRIBUTING.md#Changelog) for instructions on how to add changelog entries. -## [Unreleased 3.2](https://github.com/opensearch-project/k-NN/compare/main...HEAD) -### Infrastructure -* Bump JDK version to 24, gradle to 8.14 [#2792](https://github.com/opensearch-project/k-NN/pull/2792) -* Bump Faiss commit to 2929bf4 [#2815](https://github.com/opensearch-project/k-NN/pull/2815) -* Bump Faiss commit to 5617caa [#2824](https://github.com/opensearch-project/k-NN/pull/2824) -* Bump Gradle to 8.14.3 [#2828](https://github.com/opensearch-project/k-NN/pull/2828) - -### Features -* Support GPU indexing for FP16, Byte and Binary [#2819](https://github.com/opensearch-project/k-NN/pull/2819) - -### Enhancements -* Add random rotation feature to binary encoder for improving recall on certain datasets [#2718](https://github.com/opensearch-project/k-NN/pull/2718) -* Asymmetric Distance Computation for binary quantized faiss indices [#2733](https://github.com/opensearch-project/k-NN/pull/2733) -* Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) -* Patch for supporting nested search in IndexBinaryHNSWCagra [#2824](https://github.com/opensearch-project/k-NN/pull/2824) -* Support Asymmetric Distance Computation in Lucene-on-Faiss [#2781](https://github.com/opensearch-project/k-NN/pull/2781) - -### Bug Fixes -* [Remote Vector Index Build] Don't fall back to CPU on terminal failures [#2773](https://github.com/opensearch-project/k-NN/pull/2773) -* Fix @ collision in NativeMemoryCacheKeyHelper for vector index filenames containing @ characters [#2810](https://github.com/opensearch-project/k-NN/pull/2810) - - -## [Unreleased 3.3](https://github.com/opensearch-project/k-NN/compare/main...HEAD) +## [Unreleased 3.3](https://github.com/opensearch-project/k-NN/compare/main...HEAD) \ No newline at end of file diff --git a/build.gradle b/build.gradle index 1aa96ed7c1..00d609ea63 100644 --- a/build.gradle +++ b/build.gradle @@ -336,14 +336,13 @@ task release(type: Copy, group: 'build') { //****************************************************************************/ // Dependencies //****************************************************************************/ -// Force resolution of conflicting dependencies for tests -configurations.matching { it.name.contains('test') || it.name.contains('Test') }.all { +// Force resolution of conflicting dependencies +configurations.all { resolutionStrategy { force 'com.google.guava:guava:32.1.3-jre' force 'com.google.guava:failureaccess:1.0.1' } } - dependencies { api "org.opensearch:opensearch:${opensearch_version}" api project(":remote-index-build-client") From 501d359f14ccd10622780dfcfb98697c21ba1207 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 6 Aug 2025 19:17:36 +0000 Subject: [PATCH 12/15] filter out jacoco Signed-off-by: Karen Xu --- build.gradle | 10 ++++++---- release-notes/opensearch-k-NN.release-notes-3.2.0.0.md | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/build.gradle b/build.gradle index 00d609ea63..30186564f8 100644 --- a/build.gradle +++ b/build.gradle @@ -336,11 +336,13 @@ task release(type: Copy, group: 'build') { //****************************************************************************/ // Dependencies //****************************************************************************/ -// Force resolution of conflicting dependencies -configurations.all { +// Force resolution of conflicting dependencies globally +configurations.matching { + it.canBeResolved && !it.name.toLowerCase().contains('jacoco') +}.all { resolutionStrategy { - force 'com.google.guava:guava:32.1.3-jre' - force 'com.google.guava:failureaccess:1.0.1' + force 'com.google.guava:guava:32.1.3-jre' // Override transport-grpc's 33.2.1-jre + force 'com.google.guava:failureaccess:1.0.1' // Override transport-grpc's 1.0.2 } } dependencies { diff --git a/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md b/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md index 5c258fb412..f55105da22 100644 --- a/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md +++ b/release-notes/opensearch-k-NN.release-notes-3.2.0.0.md @@ -12,7 +12,7 @@ Compatible with OpenSearch 3.2.0 * Support GPU indexing for FP16, Byte and Binary [#2819](https://github.com/opensearch-project/k-NN/pull/2819) * Add random rotation feature to binary encoder for improving recall on certain datasets [#2718](https://github.com/opensearch-project/k-NN/pull/2718) * Asymmetric Distance Computation (ADC) for binary quantized faiss indices [#2733](https://github.com/opensearch-project/k-NN/pull/2733) -* Extend transport-grpc module to support GRPC KNN queries [#2792](https://github.com/opensearch-project/k-NN/pull/2792) +* Extend transport-grpc module to support GRPC KNN queries [#2817](https://github.com/opensearch-project/k-NN/pull/2817) ### Enhancements * Add KNN timing info to core profiler [#2785](https://github.com/opensearch-project/k-NN/pull/2785) From 86ec89dee8546ed80003bb73862d4fd620543137 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 7 Aug 2025 00:54:12 +0000 Subject: [PATCH 13/15] exclude guava and failure access from transport-grpc Signed-off-by: Karen Xu --- build.gradle | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/build.gradle b/build.gradle index 30186564f8..ff7b89fcab 100644 --- a/build.gradle +++ b/build.gradle @@ -315,7 +315,7 @@ opensearchplugin { // zip file name and plugin name in ${opensearch.plugin.name} read by OpenSearch when plugin loading description 'OpenSearch k-NN plugin' classname 'org.opensearch.knn.plugin.KNNPlugin' - extendedPlugins = ['lang-painless'] + extendedPlugins = ['lang-painless', 'transport-grpc'] licenseFile = rootProject.file('LICENSE.txt') noticeFile = rootProject.file('NOTICE.txt') } @@ -336,20 +336,14 @@ task release(type: Copy, group: 'build') { //****************************************************************************/ // Dependencies //****************************************************************************/ -// Force resolution of conflicting dependencies globally -configurations.matching { - it.canBeResolved && !it.name.toLowerCase().contains('jacoco') -}.all { - resolutionStrategy { - force 'com.google.guava:guava:32.1.3-jre' // Override transport-grpc's 33.2.1-jre - force 'com.google.guava:failureaccess:1.0.1' // Override transport-grpc's 1.0.2 - } -} dependencies { api "org.opensearch:opensearch:${opensearch_version}" api project(":remote-index-build-client") compileOnly "org.opensearch.plugin:opensearch-scripting-painless-spi:${versions.opensearch}" - compileOnly "org.opensearch.plugin:transport-grpc:${opensearch_version}" + compileOnly("org.opensearch.plugin:transport-grpc:${opensearch_version}") { + exclude group: 'com.google.guava', module: 'guava' + exclude group: 'com.google.guava', module: 'failureaccess' + } compileOnly "org.opensearch:protobufs:0.6.0" api group: 'com.google.guava', name: 'failureaccess', version:'1.0.1' api group: 'com.google.guava', name: 'guava', version:'32.1.3-jre' From 3b2ad7cc6e8ae39ea67829a776facde791cba937 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 7 Aug 2025 20:10:17 +0000 Subject: [PATCH 14/15] upgrade failureaccess and guava to match core, and make guava a test only dep Signed-off-by: Karen Xu --- build.gradle | 9 +++++++-- qa/build.gradle | 2 ++ qa/restart-upgrade/build.gradle | 7 ++++++- qa/rolling-upgrade/build.gradle | 6 +++++- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/build.gradle b/build.gradle index ff7b89fcab..3247ec495e 100644 --- a/build.gradle +++ b/build.gradle @@ -340,15 +340,20 @@ dependencies { api "org.opensearch:opensearch:${opensearch_version}" api project(":remote-index-build-client") compileOnly "org.opensearch.plugin:opensearch-scripting-painless-spi:${versions.opensearch}" + // TODO migrate this to a dependency to 'transport-grpc-spi' once it is ready, to avoid excluding all these unnecessary dependencies compileOnly("org.opensearch.plugin:transport-grpc:${opensearch_version}") { exclude group: 'com.google.guava', module: 'guava' exclude group: 'com.google.guava', module: 'failureaccess' + exclude group: 'com.google.errorprone', module: 'error_prone_annotations' } compileOnly "org.opensearch:protobufs:0.6.0" - api group: 'com.google.guava', name: 'failureaccess', version:'1.0.1' - api group: 'com.google.guava', name: 'guava', version:'32.1.3-jre' + compileOnly group: 'com.google.guava', name: 'failureaccess', version:'1.0.2' + compileOnly group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' api group: 'commons-lang', name: 'commons-lang', version: '2.6' testFixturesImplementation "org.opensearch.test:framework:${opensearch_version}" + // Add Guava for test configurations since it's needed for testing but excluded from runtime + testImplementation group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' + testFixturesImplementation group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' testImplementation group: 'net.bytebuddy', name: 'byte-buddy', version: "${versions.bytebuddy}" testImplementation group: 'org.objenesis', name: 'objenesis', version: '3.3' testImplementation group: 'net.bytebuddy', name: 'byte-buddy-agent', version: "${versions.bytebuddy}" diff --git a/qa/build.gradle b/qa/build.gradle index 34cd484d7e..d20505e61b 100644 --- a/qa/build.gradle +++ b/qa/build.gradle @@ -25,6 +25,8 @@ dependencies { api "org.apache.logging.log4j:log4j-api:${versions.log4j}" api "org.apache.logging.log4j:log4j-core:${versions.log4j}" + // Guava needed for BWC test compilation (ImmutableList, ImmutableMap usage) + testImplementation group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' testImplementation "org.opensearch.test:framework:${opensearch_version}" testImplementation(testFixtures(rootProject)) } diff --git a/qa/restart-upgrade/build.gradle b/qa/restart-upgrade/build.gradle index 7775c89f15..4188232485 100644 --- a/qa/restart-upgrade/build.gradle +++ b/qa/restart-upgrade/build.gradle @@ -8,6 +8,11 @@ import org.apache.tools.ant.taskdefs.condition.Os apply from : "$rootDir/qa/build.gradle" +dependencies { + // Ensure Guava is available for BWC test compilation + testImplementation group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' +} + String default_bwc_version = System.getProperty("bwc.version") String knn_bwc_version = System.getProperty("tests.bwc.version", default_bwc_version) boolean isSnapshot = knn_bwc_version.contains("-SNAPSHOT") @@ -137,7 +142,7 @@ testClusters { excludeTestsMatching "org.opensearch.knn.bwc.ScriptScoringIT.testNonKNNIndex_withMethodParams_withLuceneEngine" } } - + if (knn_bwc_version.startsWith("1.") || knn_bwc_version.startsWith("2.0.") || knn_bwc_version.startsWith("2.1.") || diff --git a/qa/rolling-upgrade/build.gradle b/qa/rolling-upgrade/build.gradle index 746025545b..e9ae0d5ea1 100644 --- a/qa/rolling-upgrade/build.gradle +++ b/qa/rolling-upgrade/build.gradle @@ -8,6 +8,11 @@ import org.apache.tools.ant.taskdefs.condition.Os apply from : "$rootDir/qa/build.gradle" +dependencies { + // Ensure Guava is available for BWC test compilation + testImplementation group: 'com.google.guava', name: 'guava', version:'33.2.1-jre' +} + String default_bwc_version = System.getProperty("bwc.version") String knn_bwc_version = System.getProperty("tests.bwc.version", default_bwc_version) boolean isSnapshot = knn_bwc_version.contains("-SNAPSHOT") @@ -106,4 +111,3 @@ task testRollingUpgrade(type: StandaloneRestIntegTestTask) { nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}") systemProperty 'tests.security.manager', 'false' } - From 72c583274ce39141648176492dd7f72aea8e3d2e Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 7 Aug 2025 22:19:17 +0000 Subject: [PATCH 15/15] fix unit test Signed-off-by: Karen Xu --- .../KNNQueryBuilderProtoConverterTests.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverterTests.java b/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverterTests.java index cdcea4fa84..7105af93f2 100644 --- a/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverterTests.java +++ b/src/test/java/org/opensearch/knn/grpc/proto/request/search/query/KNNQueryBuilderProtoConverterTests.java @@ -40,14 +40,27 @@ public void testFromProto_validQuery() { when(queryContainer.getQueryContainerCase()).thenReturn(QueryContainer.QueryContainerCase.KNN); when(queryContainer.getKnn()).thenReturn(knnQuery); - // Mock the KNNQueryBuilderProtoUtils.fromProto method using PowerMock - KNNQueryBuilder expectedQueryBuilder = mock(KNNQueryBuilder.class); + // Mock the KnnQuery to provide required fields + when(knnQuery.getField()).thenReturn("test_field"); + when(knnQuery.getVectorList()).thenReturn(java.util.Arrays.asList(1.0f, 2.0f, 3.0f)); + when(knnQuery.getK()).thenReturn(10); + + // Mock optional fields that may be checked + when(knnQuery.hasMaxDistance()).thenReturn(false); + when(knnQuery.hasMinScore()).thenReturn(false); + when(knnQuery.hasMethodParameters()).thenReturn(false); + when(knnQuery.hasFilter()).thenReturn(false); + when(knnQuery.hasRescore()).thenReturn(false); + when(knnQuery.hasBoost()).thenReturn(false); + when(knnQuery.hasUnderscoreName()).thenReturn(false); + when(knnQuery.hasExpandNestedDocs()).thenReturn(false); // Test QueryBuilder result = converter.fromProto(queryContainer); // Verify assertNotNull(result); + assertTrue(result instanceof KNNQueryBuilder); } @Test