[GRPC] Extend transport-grpc module to support GRPC KNN queries

[karenyrx]

Description

1. Extend the transport-grpc module to support GRPC-based KNN queries.
2. Upgrade the guava version to avoid dependency conflicts with the opensearch-protobufs library.

Note: This PR is dependent on OpenSearch PR #18897 moving the transport-grpc plugin to a module.

Future enhancements

In a future PR, an integration test for GRPC KNN will be added. This requires an OpenSearchGrpcTestCase test fixture is added to OpenSearch core, similar to the existing OpenSearchRestTestCase.

Related Issues

Part of #2816

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[navneet1v]

@karenyrx can you please publish the PR and also see why tests are failing. It is saying some package is missing

[karenyrx]

Thanks for taking a look. Per the PR description, this PR will fail until opensearch-project/OpenSearch#18897 is merged. I've just created the PR to share it out for early review.

I was thinking of marking it ready for review once it's tested after the other PR is merged.
Would you like me to publish it for review right now?

[navneet1v]

please publish the PR that will help

[navneet1v]

In a future PR, an integration test for GRPC KNN will be added. This requires an OpenSearchGrpcTestCase test fixture is added to OpenSearch core, similar to the existing OpenSearchRestTestCase.

Can we create a GH issue for this task. This will ensure that effort to add ITs is not lost.

We should also add the BWC test for the same.

[navneet1v]

Mostly code looks good to me. Some minor comments.

Also adding the discussion I had with @finnegancarroll here:

1. The protobuff endpoint will be present for newer OpenSearch clusters.
2. If there is a mix cluster case then responsibility of sending the protobuff based search request to right nodes is on operator.
3. Node to Node communication is still happing via REST.

[karenyrx]

Latest changes require opensearch-project/OpenSearch#18923 to be merged

[karenyrx]

After PR was merged, confirmed local compile is working:

~/k-NN on [knnpr2] % cd /home/user/k-NN && export JAVA_HOME=/opt/jvm/jdk-21 && export PATH=$JAVA_HOME/bin:$PATH && ./gradlew compileJava --parallel       
....

BUILD SUCCESSFUL in 51s
4 actionable tasks: 3 executed, 1 up-to-date

[karenyrx]

Can we create a GH issue for this task. This will ensure that effort to add ITs is not lost.

We should also add the BWC test for the same.

Added it to the "Milestones" section in this issue: #2816

[shatejas]

Looks good

Waiting for the CIs to pass before approving

[navneet1v]

@karenyrx some minor comments structural comments nothing major, I think PR is ready for merge once we have successful CI runs.

[karenyrx]

After excluding guava and failure-access from the transport-grpc dep manually, built a knn distrib locally and confirmed guava and failure access are both still in the bundle -

Build commands:

export JAVA_HOME=/opt/jvm/jdk-21 && export PATH=$JAVA_HOME/bin:$PATH && ./gradlew clean bundlePlugin --no-daemon -q

Contents of the zip:

~/k-NN on [knnpr2] % unzip -l build/distributions/opensearch-knn-3.2.0.0-SNAPSHOT.zip
Archive:  build/distributions/opensearch-knn-3.2.0.0-SNAPSHOT.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
     1929  1980-02-01 00:00   plugin-descriptor.properties
      700  1980-02-01 00:00   plugin-security.policy
  1069010  1980-02-01 00:00   opensearch-knn-3.2.0.0-SNAPSHOT.jar
    32198  1980-02-01 00:00   remote-index-build-client-3.2.0.0-SNAPSHOT.jar
     4617  1980-02-01 00:00   failureaccess-1.0.1.jar
  3043932  1980-02-01 00:00   guava-32.1.3-jre.jar
   284220  1980-02-01 00:00   commons-lang-2.6.jar
  1375665  1980-02-01 00:00   jna-platform-5.16.0.jar
   979563  1980-02-01 00:00   oshi-core-6.4.13.jar
    41125  1980-02-01 00:00   slf4j-api-1.7.36.jar
   911620  1980-02-01 00:00   httpclient5-5.4.4.jar
   908601  1980-02-01 00:00   httpcore5-5.3.4.jar
   241692  1980-02-01 00:00   httpcore5-h2-5.3.4.jar
       51  1980-02-01 00:00   NOTICE.txt
    20535  1980-02-01 00:00   LICENSE.txt
---------                     -------
  8915458                     15 files

Could a maintainer help approve the CI to confirm the issue is fixed?

[navneet1v]

@karenyrx is the PR to fix the dependency of grpc plugin is merged? the latest failure tells me that the change of grpc is not merged as of now

[karenyrx]

BWC checks are passing now. Pushing a new commit to fix the failing unit tests.

Tests with failures:
 - org.opensearch.knn.grpc.proto.request.search.query.KNNQueryBuilderProtoConverterTests.testFromProto_validQuery

> Task :test FAILED

[navneet1v]

pasting the followup plan as suggested by @karenyrx on the slack.

Followup I plan to take on after this PR is merged:

1. Short term: Make sure all the plugins that are dependent on KNN ( e.g. ML commons, Security Analytics, and Neural Search) can still be built + run.
   1. If they are using guava as a compileOnly dependency, they will have to be changed to be an implementation instead.
   2. Please correct if I missed any plugins that are dependent on KNN
2. Medium term: Figure out how to write integration + BWC tests for this
3. Long term: Migrate to use the transport-grpc SPI approach.
   Benefit: No more unecessary deps introduced to KNN plugin by the transport-grpc plugin (e.g. guava)
4. Optional: Up to KNN maintainers whether the guava dep should be removed from KNN plugin or not - which would require a migration of all the Immutable collections to the JDK ones

cc: @vamshin , @shatejas , @finnegancarroll , @andrross

[navneet1v]

@shatejas , @jmazanec15 can one of you take a look at this PR?

[owaiskazi19]

Neural search build is failing

> Task :run FAILED
Exec output and error:
| Output for ./bin/opensearch-plugin:-> Installing file:/Users/kazabdu/.gradle/caches/modules-2/files-2.1/org.opensearch.plugin/opensearch-job-scheduler/3.2.0.0-SNAPSHOT/5d7eb58e5c0fbdf4796b61fb200f17aac7b76825/opensearch-job-scheduler-3.2.0.0-SNAPSHOT.zip
| -> Downloading file:/Users/kazabdu/.gradle/caches/modules-2/files-2.1/org.opensearch.plugin/opensearch-job-scheduler/3.2.0.0-SNAPSHOT/5d7eb58e5c0fbdf4796b61fb200f17aac7b76825/opensearch-job-scheduler-3.2.0.0-SNAPSHOT.zip
| -> Installed opensearch-job-scheduler with folder name opensearch-job-scheduler
| -> Installing file:/Users/kazabdu/.gradle/caches/modules-2/files-2.1/org.opensearch.plugin/opensearch-knn/3.2.0.0-SNAPSHOT/58e1b6a3e8e22b6ecf6a3f82cef5829a73a21ba2/opensearch-knn-3.2.0.0-SNAPSHOT.zip
| -> Downloading file:/Users/kazabdu/.gradle/caches/modules-2/files-2.1/org.opensearch.plugin/opensearch-knn/3.2.0.0-SNAPSHOT/58e1b6a3e8e22b6ecf6a3f82cef5829a73a21ba2/opensearch-knn-3.2.0.0-SNAPSHOT.zip
| -> Failed installing file:/Users/kazabdu/.gradle/caches/modules-2/files-2.1/org.opensearch.plugin/opensearch-knn/3.2.0.0-SNAPSHOT/58e1b6a3e8e22b6ecf6a3f82cef5829a73a21ba2/opensearch-knn-3.2.0.0-SNAPSHOT.zip
| -> Rolling back opensearch-job-scheduler
| -> Rolled back opensearch-job-scheduler
| -> Rolling back file:/Users/kazabdu/.gradle/caches/modules-2/files-2.1/org.opensearch.plugin/opensearch-knn/3.2.0.0-SNAPSHOT/58e1b6a3e8e22b6ecf6a3f82cef5829a73a21ba2/opensearch-knn-3.2.0.0-SNAPSHOT.zip
| -> Rolled back file:/Users/kazabdu/.gradle/caches/modules-2/files-2.1/org.opensearch.plugin/opensearch-knn/3.2.0.0-SNAPSHOT/58e1b6a3e8e22b6ecf6a3f82cef5829a73a21ba2/opensearch-knn-3.2.0.0-SNAPSHOT.zip
| Exception in thread "main" java.lang.IllegalArgumentException: Missing plugin [transport-grpc], dependency of [opensearch-knn]
|       at org.opensearch.plugins.PluginsService.addSortedBundle(PluginsService.java:596)
|       at org.opensearch.plugins.PluginsService.sortBundles(PluginsService.java:559)
|       at org.opensearch.plugins.PluginsService.checkJarHellForPlugin(PluginsService.java:403)
|       at org.opensearch.tools.cli.plugin.InstallPluginCommand.jarHellCheck(InstallPluginCommand.java:834)
|       at org.opensearch.tools.cli.plugin.InstallPluginCommand.loadPluginInfo(InstallPluginCommand.java:811)
|       at org.opensearch.tools.cli.plugin.InstallPluginCommand.installPlugin(InstallPluginCommand.java:846)
|       at org.opensearch.tools.cli.plugin.InstallPluginCommand.execute(InstallPluginCommand.java:277)
|       at org.opensearch.tools.cli.plugin.InstallPluginCommand.execute(InstallPluginCommand.java:251)
|       at org.opensearch.common.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:110)
|       at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
|       at org.opensearch.cli.MultiCommand.execute(MultiCommand.java:104)
|       at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
|       at org.opensearch.cli.Command.main(Command.java:101)
|       at org.opensearch.tools.cli.plugin.PluginCli.main(PluginCli.java:66)

[karenyrx]

Confirmed that the neural-search ./gradlew check completed successfully:

BUILD SUCCESSFUL in 21m 24s
91 actionable tasks: 91 executed

The ongoing opensearch-build RCs are also succeeding so far: https://build.ci.opensearch.org/blue/organizations/jenkins/distribution-build-opensearch/detail/distribution-build-opensearch/11325/pipeline
https://build.ci.opensearch.org/blue/organizations/jenkins/distribution-build-opensearch[…]il/distribution-build-opensearch-dashboards/8587/pipeline

The error above is potentially a caching issue where local Gradle caches were still using an older version of opensearch core where transport-grpc had not been moved to a module yet. After the core PR to move the transport-grpc plugin to a module , transport-grpc should be auto-installed/bundled with all opensearch core distributions, so it should not be unavailable. ./gradlew clean && rm -rf ~/.gradle && rm -rf ~/.m2 may be needed.