throw exception if remote model doesn't return 2xx status code; fix p…

[ylwu-amzn]

…dict runner

Description

1. Throw exception if remote model doesn't return 2xx status code, for example invalid credential, exceed throttling limit.
2. Fix predict runner: if predict is null, will directly return, otherwise will try to read model and predict,
3. Check if task is in cache or not before updating to avoid "Task not found" error.
4. Support generate embeddings for multiple docs for remote embedding model. Which was disabled by this PR Add neural search default processor for non OpenAI/Cohere scenario #1274

Test

bulk ingestion with invalid credential

bulk ingestion with throttling error

Reindex data from raw text to KNN index

neural search

neural search with exception

Issues Resolved

[List any issues this PR will resolve]

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed per the DCO using --signoff

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.

[codecov]

Codecov Report

Merging #1473 (d9c920c) into 2.9 (5fc555d) will decrease coverage by 0.02%.
The diff coverage is 44.44%.

❗ Current head d9c920c differs from pull request most recent head c52b1bf. Consider uploading reports for the commit c52b1bf to get more accurate results

@@             Coverage Diff              @@
##                2.9    #1473      +/-   ##
============================================
- Coverage     78.73%   78.71%   -0.02%     
- Complexity     2163     2166       +3     
============================================
  Files           168      168              
  Lines          8784     8792       +8     
  Branches        879      882       +3     
============================================
+ Hits           6916     6921       +5     
- Misses         1470     1471       +1     
- Partials        398      400       +2     

Flag	Coverage Δ
ml-commons	78.71% <44.44%> (-0.02%)	⬇️

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

Files	Coverage Δ
...engine/algorithms/remote/AwsConnectorExecutor.java	81.81% <50.00%> (-1.21%)	⬇️
...ch/ml/engine/algorithms/remote/ConnectorUtils.java	59.78% <0.00%> (ø)
...e/algorithms/remote/HttpJsonConnectorExecutor.java	66.66% <75.00%> (ø)
...n/java/org/opensearch/ml/model/MLModelManager.java	63.36% <0.00%> (ø)
...va/org/opensearch/ml/task/MLPredictTaskRunner.java	82.31% <40.00%> (-0.33%)	⬇️

[navneet1v]

Have we tested with the clients that they are able to handle the exception and throw the right exception to customer?

[ylwu-amzn]

Have we tested with the clients that they are able to handle the exception and throw the right exception to customer?

Yes , check the screenshot for bulk ingestion, which will invoke neural-search ingest process.

[navneet1v]

@ylwu-amzn can we test few of these cases:

1. Can we test the search behavior also
2. Can we also test a reindex usecase here.

[navneet1v]

is there a reason why are not throwing exception for 201 or other status code? 200 seems to be the only case in which we should not throw exception.

Or another way to ask is can a remote model give 201 status code?

[ylwu-amzn]

That depends on the remote model service to determine return 200 or other 2xx code.

Check HTTP RFC

2xx (Successful): The request was successfully received,
understood, and accepted

[navneet1v]

So we are treating all 2XX as successful response.

[ylwu-amzn]

Yes, because that's what HTTP RFC defines

[ylwu-amzn]

@ylwu-amzn can we test few of these cases:

1. Can we test the search behavior also
2. Can we also test a reindex usecase here.

Done, check the description part

[ylwu-amzn]

@ylwu-amzn can we test few of these cases:

1. Can we test the search behavior also
2. Can we also test a reindex usecase here.

Done, check the description part

I don't see the error image with neural search search query.

Test with same model , bulk ingest will throw error

{
  "errors": true,
  "ingest_took": 97,
  "items": [
    {
      "index": {
        "_id": null,
        "_index": "semantic_demostore_ylwu_cohere",
        "error": {
          "reason": "{\"message\":\"You are using a Trial key, which is limited to 1000 API calls / month. You can continue to use the Trial key for free or upgrade to a Production key with higher rate limits at 'https://dashboard.cohere.ai/api-keys'. Contact us on 'https://discord.gg/XW44jPfYJu' or email us at support@cohere.com with any questions\"}",
          "type": "status_exception"
        },
        "status": 429
      }
    },

Reindex does not

{
  "batches": 1,
  "created": 3,
  "deleted": 0,
  "failures": [],
  "noops": 0,
  "requests_per_second": -1.0,
  "retries": {
    "bulk": 1,
    "search": 0
  },
  "throttled_millis": 0,
  "throttled_until_millis": 0,
  "timed_out": false,
  "took": 694,
  "total": 3,
  "updated": 0,
  "version_conflicts": 0
}

Need neural-search team to help deep dive more. ml-commons does throw out exception as you can see in bulk ingestion result, but reindex part not show the error message

[ylwu-amzn]

Also is it expected that github workflows will fail?

Fixed. local build passed, let's see Github CI can pass or not

[navneet1v]

@ylwu-amzn can we test few of these cases:

1. Can we test the search behavior also
2. Can we also test a reindex usecase here.

Done, check the description part

I don't see the error image with neural search search query.

Test with same model , bulk ingest will throw error

{
  "errors": true,
  "ingest_took": 97,
  "items": [
    {
      "index": {
        "_id": null,
        "_index": "semantic_demostore_ylwu_cohere",
        "error": {
          "reason": "{\"message\":\"You are using a Trial key, which is limited to 1000 API calls / month. You can continue to use the Trial key for free or upgrade to a Production key with higher rate limits at 'https://dashboard.cohere.ai/api-keys'. Contact us on 'https://discord.gg/XW44jPfYJu' or email us at support@cohere.com with any questions\"}",
          "type": "status_exception"
        },
        "status": 429
      }
    },

Reindex does not

{
  "batches": 1,
  "created": 3,
  "deleted": 0,
  "failures": [],
  "noops": 0,
  "requests_per_second": -1.0,
  "retries": {
    "bulk": 1,
    "search": 0
  },
  "throttled_millis": 0,
  "throttled_until_millis": 0,
  "timed_out": false,
  "took": 694,
  "total": 3,
  "updated": 0,
  "version_conflicts": 0
}

Need neural-search team to help deep dive more. ml-commons does throw out exception as you can see in bulk ingestion result, but reindex part not show the error message

The way you tested with the successful query can we test with the error being thrown from ML commons when request for predict api is for search.

[navneet1v]

@ylwu-amzn can we test few of these cases:

1. Can we test the search behavior also
2. Can we also test a reindex usecase here.

Done, check the description part

I don't see the error image with neural search search query.

Test with same model , bulk ingest will throw error

{
  "errors": true,
  "ingest_took": 97,
  "items": [
    {
      "index": {
        "_id": null,
        "_index": "semantic_demostore_ylwu_cohere",
        "error": {
          "reason": "{\"message\":\"You are using a Trial key, which is limited to 1000 API calls / month. You can continue to use the Trial key for free or upgrade to a Production key with higher rate limits at 'https://dashboard.cohere.ai/api-keys'. Contact us on 'https://discord.gg/XW44jPfYJu' or email us at support@cohere.com with any questions\"}",
          "type": "status_exception"
        },
        "status": 429
      }
    },

Reindex does not

{
  "batches": 1,
  "created": 3,
  "deleted": 0,
  "failures": [],
  "noops": 0,
  "requests_per_second": -1.0,
  "retries": {
    "bulk": 1,
    "search": 0
  },
  "throttled_millis": 0,
  "throttled_until_millis": 0,
  "timed_out": false,
  "took": 694,
  "total": 3,
  "updated": 0,
  "version_conflicts": 0
}

Need neural-search team to help deep dive more. ml-commons does throw out exception as you can see in bulk ingestion result, but reindex part not show the error message

The way you tested with the successful query can we test with the error being thrown from ML commons when request for predict api is for search.

uploading from chat with @ylwu-amzn

[opensearch-trigger-bot]

The backport to 2.11 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.11 2.11
# Navigate to the new working tree
cd .worktrees/backport-2.11
# Create a new branch
git switch --create backport/backport-1473-to-2.11
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 513ca39cafba612b93cd19473b7e83c3c6670032
# Push it to GitHub
git push --set-upstream origin backport/backport-1473-to-2.11
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.11

Then, create a pull request where the base branch is 2.11 and the compare/head branch is backport/backport-1473-to-2.11.