Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if ml-commons plugin is installed in the Cluster #480

Open
owaiskazi19 opened this issue Oct 30, 2023 · 3 comments
Open

Check if ml-commons plugin is installed in the Cluster #480

owaiskazi19 opened this issue Oct 30, 2023 · 3 comments
Assignees
Labels
enhancement help wanted Extra attention is needed Maintenance Add support for new versions of OpenSearch/Dashboards from upstream

Comments

@owaiskazi19
Copy link
Member

What is the bug?

Neural-search is using ml-commons APIs here. There can be a case when the cluster doesn't have ml-commons installed and can return in failure of these APIs on neural-search side.

How can one reproduce the bug?

  1. Have a cluster running with just neural-search plugin installed.

What is the expected behavior?

A check should be added in build.gradle here for ml-commons plugin.

What is your host/environment?

Ubuntu

Do you have any screenshots?

If applicable, add screenshots to help explain your problem.

Do you have any additional context?

Add any other context about the problem.

@owaiskazi19 owaiskazi19 added bug Something isn't working untriaged labels Oct 30, 2023
@navneet1v navneet1v added enhancement and removed untriaged bug Something isn't working labels Oct 30, 2023
@navneet1v navneet1v changed the title [BUG] Check if ml-commons plugin is installed in the Cluster Check if ml-commons plugin is installed in the Cluster Dec 5, 2023
@navneet1v navneet1v added the Maintenance Add support for new versions of OpenSearch/Dashboards from upstream label Dec 5, 2023
@navneet1v navneet1v added the good first issue Good for newcomers label Jul 29, 2024
@heemin32 heemin32 moved this to Backlog(Hot) in Neural Search RoadMap Dec 26, 2024
@heemin32 heemin32 closed this as completed by moving to Backlog(Hot) in Neural Search RoadMap Dec 26, 2024
@heemin32 heemin32 reopened this Dec 26, 2024
@junqiu-lei
Copy link
Member

In order to have ml-common dependency check for neural search plugin during OpenSearch cluster plugin installation, I tried to add opensearch-ml as extendedPlugin in neural search gralde.build here.

After this change, it causes jar hell from org.opensearch.plugins.PluginsService.checkBundleJarHell:

| Caused by: java.lang.IllegalStateException: jar hell!
| class: com.google.common.annotations.Beta
| jar1: /local/home/junqiu/dev/neural-search/build/testclusters/integTest-0/distro/3.0.0-ARCHIVE/plugins/opensearch-ml/guava-32.1.3-jre.jar
| jar2: /local/home/junqiu/dev/neural-search/build/testclusters/integTest-0/distro/3.0.0-ARCHIVE/plugins/opensearch-knn/guava-32.1.3-jre.jar

k-NN and ML each ship Guava JAR, leading to jar-hell check failure. Because OpenSearch sees two copies of com.google.common.*, it fails with a jar-hell error. Unfortunately, I tried to relocate Guava in our own plugin, but it still leads to same jar hell from k-NN and ML.

Besides I don't find any method to disable jarhell check within plugin level. That might be the reason previously only k-NN plugin is added in the extendedPlugin list.

@heemin32 heemin32 removed good first issue Good for newcomers untriaged labels Jan 3, 2025
@heemin32 heemin32 added the help wanted Extra attention is needed label Jan 3, 2025
@heemin32
Copy link
Collaborator

heemin32 commented Jan 3, 2025

Here are a few alternatives to defining both k-NN and ml-common in the extendedPlugins parameter:

  1. Eliminate the Guava dependency from either the k-NN or ml-common plugin to avoid the conflict.
  2. Verify the installation of ml-common and k-NN when the neural-search plugin is loaded, using a different approach instead of relying on the extendedPlugins parameter.
  3. Attempt to use shadowJar in both the k-NN and ml-common plugins to isolate the Guava JAR for each plugin.

It's important to note that the extendedPlugins parameter shouldn't be used solely to declare required plugins, as the neural-search plugin does not extend ml-common or k-NN through SPI. For reference, see the java comment on the parameter. https://github.com/opensearch-project/OpenSearch/blob/845fbfa10407f3264e6aab8812eff4ef0ad8be24/server/src/main/java/org/opensearch/plugins/PluginInfo.java#L103

I believe using shadowJar would be the best option to prevent future conflicts easily. If not, option 2 could be the next preferable approach, considering implementation and maintenance efforts.

@peterzhuamazon
Copy link
Member

Here are a few alternatives to defining both k-NN and ml-common in the extendedPlugins parameter:

1. Eliminate the Guava dependency from either the k-NN or ml-common plugin to avoid the conflict.

2. Verify the installation of ml-common and k-NN when the neural-search plugin is loaded, using a different approach instead of relying on the `extendedPlugins` parameter.

3. Attempt to use `shadowJar` in both the k-NN and ml-common plugins to isolate the Guava JAR for each plugin.

It's important to note that the extendedPlugins parameter shouldn't be used solely to declare required plugins, as the neural-search plugin does not extend ml-common or k-NN through SPI. For reference, see the java comment on the parameter. https://github.com/opensearch-project/OpenSearch/blob/845fbfa10407f3264e6aab8812eff4ef0ad8be24/server/src/main/java/org/opensearch/plugins/PluginInfo.java#L103

I believe using shadowJar would be the best option to prevent future conflicts easily. If not, option 2 could be the next preferable approach, considering implementation and maintenance efforts.

Was discussing about this and we probably can also do a verification upon plugin installation of neural, and check if ml+knn are already installed.

Tho this approach would need to inject scripts for both nix* and Windows env so might not be the best idea.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted Extra attention is needed Maintenance Add support for new versions of OpenSearch/Dashboards from upstream
Projects
Status: Backlog(Hot)
Development

No branches or pull requests

5 participants