Fix test failure after lucene upgrade to 10 by zane-neo · Pull Request #3426 · opensearch-project/ml-commons

zane-neo · 2025-01-23T08:20:45Z

Description

After this change: opensearch-project/OpenSearch#16366, the TotalHits's value is been moved to a private field, unable to access it directly outside that class, so removing this to fix the test failure.

We need to change the .value to .value() to make sure the code runs correctly.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

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.

Signed-off-by: zane-neo <zaniu@amazon.com>

peterzhuamazon · 2025-01-23T16:18:34Z

build.gradle

            opensearch_build += "-SNAPSHOT"
        }

-        common_utils_version = System.getProperty("common_utils.version", opensearch_build)


This should stay on opensearch_build as it will be switched to 3.0.0.0-alpha1-SNAPSHOT as the version of common-utils.

@peterzhuamazon I didn't see the 3.0.0.0-alpha1-SNAPSHOT available on maven repo: https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/common-utils/, I can't compile success from my local.

it's still failing with opensearch_build for neural as well

That is because Common Utils owner needs to update their plugins 1st to be compatible with lucene10.

@zane-neo @martin-gaievski I have updated common-utils main to 3.0.0.0-alpha1-SNAPSHOT and published to sonatype:

[Release 3.0] Update common-utils shadow plugin repo and bump to 3.0.0.0-alpha1 common-utils#775 (comment)

Thanks.

peterzhuamazon · 2025-01-23T16:18:40Z

build.gradle

        isSnapshot = "true" == System.getProperty("build.snapshot", "true")
-        opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT")
-        buildVersionQualifier = System.getProperty("build.version_qualifier", "")
+        opensearch_version = System.getProperty("opensearch.version", "3.0.0-alpha1-SNAPSHOT")


This is correct.

peterzhuamazon · 2025-01-23T16:20:07Z

plugin/build.gradle

    implementation "org.opensearch.client:opensearch-rest-client:${opensearch_version}"
    // Multi-tenant SDK Client
-    implementation "org.opensearch:opensearch-remote-metadata-sdk:${opensearch_build}"
+    implementation "org.opensearch:opensearch-remote-metadata-sdk:3.0.0-SNAPSHOT"


Remote sdk should also be 3.0.0.0-alpha1-SNAPSHOT.

@dbwiddis

Same above for this: https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/opensearch-remote-metadata-sdk/

Remote sdk should also be 3.0.0.0-alpha1-SNAPSHOT.

We are still making changes on that for 2.19.0. I do not plan on updating to alpha until after at least code freeze date.

If anyone wants to get ahead of the game they are welcome to check out that repo and publish it locally with alpha1. All this churn as we're trying to finish up features within the week is causing a lot of wasted effort and delays.

@dbwiddis just an update CU main is updated to 3.0.0.0-alpha1-SNAPSHOT:

[Release 3.0] Update common-utils shadow plugin repo and bump to 3.0.0.0-alpha1 common-utils#775 (comment)

Thanks.

pyek-bot

changes for lucene upgrade (TotalHits) looks good

peterzhuamazon · 2025-02-11T19:56:51Z

Build is fixed, need to change all the rest client calls back to normal client, instead of transport.client for IT test to pass.

Think @rithin-pullela-aws will take a look on it per offline discussion.

Thanks.

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo · 2025-02-12T07:30:02Z

The failure test seems a flaky one, please review again

codecov · 2025-02-12T18:39:41Z

Codecov Report

Attention: Patch coverage is 42.10526% with 11 lines in your changes missing coverage. Please review.

Project coverage is 80.26%. Comparing base (d7dec0f) to head (e14923c).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...pensearch/ml/common/model/LocalRegexGuardrail.java	0.00%	2 Missing ⚠️
...search/ml/action/tasks/GetTaskTransportAction.java	0.00%	2 Missing ⚠️
...nsearch/ml/autoredeploy/MLModelAutoReDeployer.java	0.00%	0 Missing and 2 partials ⚠️
...earch/ml/engine/indices/MLInputDatasetHandler.java	0.00%	1 Missing ⚠️
...action/undeploy/TransportUndeployModelsAction.java	0.00%	1 Missing ⚠️
...a/org/opensearch/ml/model/MLModelGroupManager.java	0.00%	0 Missing and 1 partial ⚠️
...ava/org/opensearch/ml/processor/ModelExecutor.java	75.00%	1 Missing ⚠️
.../main/java/org/opensearch/ml/utils/IndexUtils.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3426      +/-   ##
============================================
+ Coverage     80.25%   80.26%   +0.01%     
+ Complexity     6906     6904       -2     
============================================
  Files           610      610              
  Lines         30077    30077              
  Branches       3368     3368              
============================================
+ Hits          24137    24141       +4     
+ Misses         4487     4484       -3     
+ Partials       1453     1452       -1

Flag	Coverage Δ
ml-commons	`80.26% <42.10%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rithin-pullela-aws · 2025-02-12T20:07:26Z

@zane-neo please refer to this change to address the code coverage issue

Check warning on line 130 in plugin/src/main/java/org/opensearch/ml/utils/IndexUtils.java

@codecov
codecov
/ codecov/patch
plugin/src/main/java/org/opensearch/ml/utils/IndexUtils.java#L130

Added line #L130 was not covered by tests

rithin-pullela-aws · 2025-02-12T20:09:47Z

plugin/src/main/java/org/opensearch/ml/processor/ModelExecutor.java

        Object modelOutputValue;
        if (tensor.getDataType().isInteger()) {
-            modelOutputValue = Arrays.stream(tensor.getData()).map(Number::intValue).map(Integer::new).collect(Collectors.toList());
+            modelOutputValue = Arrays.stream(tensor.getData()).map(Number::intValue).collect(Collectors.toList());


Is removing .map(Integer::new) necessary for lucene 10 changes?
(Same for all the changes in this file)

JDK21 has removed the new Integer() method, and based on the code, these conversion are not necessary so it's safe to remove all of them.

Signed-off-by: rithin-pullela-aws <rithinp@amazon.com>

peterzhuamazon · 2025-02-12T22:12:49Z

@zane-neo please refer to this change to address the code coverage issue

Check warning on line 130 in plugin/src/main/java/org/opensearch/ml/utils/IndexUtils.java
@codecov
codecov
/ codecov/patch
plugin/src/main/java/org/opensearch/ml/utils/IndexUtils.java#L130
Added line #L130 was not covered by tests

Update @rithin-pullela-aws test coverage commit to this PR.

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

peterzhuamazon · 2025-02-12T23:43:31Z

Seems like integTest is not correctly using 3.0.0-alpha1-SNAPSHOT


> Task :opensearch-ml-plugin:integTest
MLModelAutoReDeployerIT STANDARD_ERROR
    Feb 12, 2025 11:41:44 PM org.apache.lucene.internal.vectorization.VectorizationProvider lookup
    WARNING: Java vector incubator module is not readable. For optimal vector performance, pass '--add-modules jdk.incubator.vector' to enable Vector API.
MLModelAutoReDeployerIT > testModelAutoRedeploy STANDARD_OUT
    [2025-02-13T11:41:44,467][INFO ][o.o.m.a.MLModelAutoReDeployerIT] [testModelAutoRedeploy] before test
    [2025-02-13T11:41:44,588][INFO ][o.o.m.a.MLModelAutoReDeployerIT] [testModelAutoRedeploy] initializing REST clients against [http://[::1]:46321, http://127.0.0.1:43023, http://[::1]:40255, http://127.0.0.1:45239, http://[::1]:36245, http://127.0.0.1:40575]
MLModelAutoReDeployerIT > testModelAutoRedeploy STANDARD_ERROR
    feb. 13, 2025 11:42:00 DOPOŁDNJA org.opensearch.client.RestClient logResponse
    WARNING: request [DELETE http://[::1]:36245/.plugins-ml-model-group] returned 1 warnings: [299 OpenSearch-3.0.0-SNAPSHOT-d0a65d3b457e11d514171ecc0a77450fdd7d25bb "this request accesses system indices: [.plugins-ml-model-group], but in a future major version, direct access to system indices will be prevented by default"]
    feb. 13, 2025 11:42:00 DOPOŁDNJA org.opensearch.client.RestClient logResponse
    WARNING: request [DELETE http://127.0.0.1:45239/.plugins-ml-config] returned 1 warnings: [299 OpenSearch-3.0.0-SNAPSHOT-d0a65d3b457e11d514171ecc0a77450fdd7d25bb "this request accesses system indices: [.plugins-ml-config], but in a future major version, direct access to system indices will be prevented by default"]
    feb. 13, 2025 11:42:00 DOPOŁDNJA org.opensearch.client.RestClient logResponse
    WARNING: request [DELETE http://[::1]:40255/.plugins-ml-model] returned 1 warnings: [299 OpenSearch-3.0.0-SNAPSHOT-d0a65d3b457e11d514171ecc0a77450fdd7d25bb "this request accesses system indices: [.plugins-ml-model], but in a future major version, direct access to system indices will be prevented by default"]
    feb. 13, 2025 11:42:00 DOPOŁDNJA org.opensearch.client.RestClient logResponse
    WARNING: request [DELETE http://127.0.0.1:43023/.plugins-ml-task] returned 1 warnings: [299 OpenSearch-3.0.0-SNAPSHOT-d0a65d3b457e11d514171ecc0a77450fdd7d25bb "this request accesses system indices: [.plugins-ml-task], but in a future major version, direct access to system indices will be prevented by default"]

peterzhuamazon

Approve based on the windows test passing, as well as unblock other plugins that depend on ml plugin.

Thanks everyone chiming in and help!

Fix test failure after lucene upgrade to 10

7067941

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo requested review from HenryL27, Zhangxunmt, austintlee, b4sjoo, dhrubo-os, jngz-es, mingshl, model-collapse, rbhavna, xinyual and ylwu-amzn as code owners January 23, 2025 08:20

zane-neo had a problem deploying to ml-commons-cicd-env January 23, 2025 08:22 — with GitHub Actions Failure

xinyual previously approved these changes Jan 23, 2025

View reviewed changes

Fix test failure after lucene upgrade to 10

686350b

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo dismissed xinyual’s stale review via 686350b January 23, 2025 08:28

zane-neo had a problem deploying to ml-commons-cicd-env January 23, 2025 08:29 — with GitHub Actions Failure

Fix test failure after lucene version upgraded to 10

88b0e08

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo had a problem deploying to ml-commons-cicd-env January 23, 2025 08:59 — with GitHub Actions Failure

Change version to opensearch version alpha1

8b61241

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo had a problem deploying to ml-commons-cicd-env January 23, 2025 10:24 — with GitHub Actions Failure

peterzhuamazon requested changes Jan 23, 2025

View reviewed changes

peterzhuamazon mentioned this pull request Jan 23, 2025

[RELEASE] Release version 3.0.0 opensearch-project/opensearch-build#3747

Closed

66 tasks

pyek-bot reviewed Jan 23, 2025

View reviewed changes

pyek-bot approved these changes Jan 23, 2025

View reviewed changes

pyek-bot mentioned this pull request Jan 23, 2025

Added amazon rekognition as a trust endpoint #3419

Merged

4 tasks

peterzhuamazon had a problem deploying to ml-commons-cicd-env February 11, 2025 19:36 — with GitHub Actions Failure

peterzhuamazon had a problem deploying to ml-commons-cicd-env February 12, 2025 03:45 — with GitHub Actions Failure

Fix compilation error

e14923c

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo temporarily deployed to ml-commons-cicd-env February 12, 2025 06:02 — with GitHub Actions Inactive

zane-neo had a problem deploying to ml-commons-cicd-env February 12, 2025 06:02 — with GitHub Actions Failure

zane-neo temporarily deployed to ml-commons-cicd-env February 12, 2025 17:41 — with GitHub Actions Inactive

peterzhuamazon mentioned this pull request Feb 12, 2025

Lucene 10 upgrade test fixes #3530

Closed

5 tasks

zane-neo had a problem deploying to ml-commons-cicd-env February 12, 2025 18:39 — with GitHub Actions Failure

zane-neo had a problem deploying to ml-commons-cicd-env February 12, 2025 18:56 — with GitHub Actions Failure

rithin-pullela-aws reviewed Feb 12, 2025

View reviewed changes

peterzhuamazon mentioned this pull request Feb 12, 2025

[Release 3.0] Bump gradle 8.10.2 / JDK23 / 3.0.0.0-alpha1 on SQL plugin opensearch-project/sql#3319

Merged

1 task

Added test to cover getNumberOfDocumentsInIndex with 4 arguments

e79bd57

Signed-off-by: rithin-pullela-aws <rithinp@amazon.com>

peterzhuamazon temporarily deployed to ml-commons-cicd-env February 12, 2025 22:13 — with GitHub Actions Inactive

peterzhuamazon had a problem deploying to ml-commons-cicd-env February 12, 2025 22:13 — with GitHub Actions Failure

Resolve conflict with the main branch

b3194f7

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

peterzhuamazon temporarily deployed to ml-commons-cicd-env February 12, 2025 22:21 — with GitHub Actions Inactive

peterzhuamazon had a problem deploying to ml-commons-cicd-env February 12, 2025 22:21 — with GitHub Actions Failure

peterzhuamazon mentioned this pull request Feb 12, 2025

[3.0] Update neural-search for OpenSearch 3.0 compatibility opensearch-project/neural-search#1141

Merged

5 tasks

peterzhuamazon had a problem deploying to ml-commons-cicd-env February 12, 2025 23:08 — with GitHub Actions Failure

dhrubo-os approved these changes Feb 12, 2025

View reviewed changes

peterzhuamazon approved these changes Feb 13, 2025

View reviewed changes

peterzhuamazon merged commit ebc0792 into opensearch-project:main Feb 13, 2025
7 of 8 checks passed

Conversation

zane-neo commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

peterzhuamazon Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterzhuamazon Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pyek-bot left a comment

Choose a reason for hiding this comment

Uh oh!

peterzhuamazon commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zane-neo commented Feb 12, 2025

Uh oh!

codecov bot commented Feb 12, 2025

Codecov Report

Uh oh!

rithin-pullela-aws commented Feb 12, 2025

Uh oh!

rithin-pullela-aws Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zane-neo Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterzhuamazon commented Feb 12, 2025

Uh oh!

peterzhuamazon commented Feb 12, 2025

Uh oh!

peterzhuamazon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

zane-neo commented Jan 23, 2025 •

edited

Loading

peterzhuamazon Jan 23, 2025 •

edited

Loading

peterzhuamazon Jan 24, 2025 •

edited

Loading

peterzhuamazon commented Feb 11, 2025 •

edited

Loading

rithin-pullela-aws Feb 12, 2025 •

edited

Loading

zane-neo Feb 13, 2025 •

edited

Loading

peterzhuamazon left a comment •

edited

Loading