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

Change httpclient to async #1958

Merged
merged 60 commits into from
Apr 30, 2024

Conversation

zane-neo
Copy link
Collaborator

Description

Change sync httpclient to async to improve remote inference performance

Issues Resolved

#1839

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.

@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:37 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:37 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:37 — with GitHub Actions Inactive
Signed-off-by: zane-neo <[email protected]>
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:42 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:42 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:42 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:42 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:42 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 06:42 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 07:31 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 07:31 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env April 29, 2024 07:31 — with GitHub Actions Inactive
@zane-neo zane-neo merged commit 94a113d into opensearch-project:main Apr 30, 2024
12 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x 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.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1958-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 94a113da8a2d1e28a84ba7a422b43287ac00448e
# Push it to GitHub
git push --set-upstream origin backport/backport-1958-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1958-to-2.x.

zane-neo added a commit to zane-neo/ml-commons that referenced this pull request Apr 30, 2024
* Change httpclient from sync to async

Signed-off-by: zane-neo <[email protected]>

* Change from CRTAsyncHttpClient to NettyAsyncHttpClient

Signed-off-by: zane-neo <[email protected]>

* Add publisher to request

Signed-off-by: zane-neo <[email protected]>

* Change sync httpclient to async

Signed-off-by: zane-neo <[email protected]>

* Handle error case and return error response in actionLListener

Signed-off-by: zane-neo <[email protected]>

* Fix no response when exception

Signed-off-by: zane-neo <[email protected]>

* Add content type header

Signed-off-by: zane-neo <[email protected]>

* Fix issues found in functional test

Signed-off-by: zane-neo <[email protected]>

* Fix no response issue in functional test

Signed-off-by: zane-neo <[email protected]>

* fix default step size error

Signed-off-by: zane-neo <[email protected]>

* Add track inference duration for async httpclient

Signed-off-by: zane-neo <[email protected]>

* Change client appsec highlight issues implementation for async httpclient

Signed-off-by: zane-neo <[email protected]>

* Add UTs

Signed-off-by: zane-neo <[email protected]>

* Add UTs

Signed-off-by: zane-neo <[email protected]>

* Remove unused file

Signed-off-by: zane-neo <[email protected]>

* Add UTs

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Change error code to honor remote service error code

Signed-off-by: zane-neo <[email protected]>

* Add more UTs

Signed-off-by: zane-neo <[email protected]>

* Change SSRF code to make it correct for return error stattus

Signed-off-by: zane-neo <[email protected]>

* Fix failure UTs and add more UTs

Signed-off-by: zane-neo <[email protected]>

* Fix failure ITs

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Fix partial success response not correct issue

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Fix failure ITs

Signed-off-by: zane-neo <[email protected]>

* Add more UTs to increase code coverage

Signed-off-by: zane-neo <[email protected]>

* Change url regex

Signed-off-by: zane-neo <[email protected]>

* Address comments

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Fix failure UTs

Signed-off-by: zane-neo <[email protected]>

* Add UT for httpclientFactory throw exception when creating httpclient

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Address comments and add modelTensor status code

Signed-off-by: zane-neo <[email protected]>

* Address comments

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Add status code to process error response

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Rebase main after connector level http parameter support

Signed-off-by: zane-neo <[email protected]>

* Fix UT

Signed-off-by: zane-neo <[email protected]>

* Change error message when remote model return empty and chaange the behavior when one of the requests fails

Signed-off-by: zane-neo <[email protected]>

* Add comments\

Signed-off-by: zane-neo <[email protected]>

* Remove redundant builder and change the error code check

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Add more UTs for throw exception cases

Signed-off-by: zane-neo <[email protected]>

* fix failure UTs

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Fix test cases since the error message change

Signed-off-by: zane-neo <[email protected]>

* Rebase code

Signed-off-by: zane-neo <[email protected]>

* fix failure IT

Signed-off-by: zane-neo <[email protected]>

* Add more UTs

Signed-off-by: zane-neo <[email protected]>

* Fix duplicate response to client issue

Signed-off-by: zane-neo <[email protected]>

* fix duplicate response in channel

Signed-off-by: zane-neo <[email protected]>

* change code for all successfully responses case

Signed-off-by: zane-neo <[email protected]>

* Address comments

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

* Increase nio httpclient version to fix vulnerbility

Signed-off-by: zane-neo <[email protected]>

* Change validate localhost logic to same with existing code

Signed-off-by: zane-neo <[email protected]>

* change method signature to private

Signed-off-by: zane-neo <[email protected]>

* format code

Signed-off-by: zane-neo <[email protected]>

---------

Signed-off-by: zane-neo <[email protected]>
zane-neo added a commit that referenced this pull request Apr 30, 2024
* Change httpclient from sync to async



* Change from CRTAsyncHttpClient to NettyAsyncHttpClient



* Add publisher to request



* Change sync httpclient to async



* Handle error case and return error response in actionLListener



* Fix no response when exception



* Add content type header



* Fix issues found in functional test



* Fix no response issue in functional test



* fix default step size error



* Add track inference duration for async httpclient



* Change client appsec highlight issues implementation for async httpclient



* Add UTs



* Add UTs



* Remove unused file



* Add UTs



* format code



* Change error code to honor remote service error code



* Add more UTs



* Change SSRF code to make it correct for return error stattus



* Fix failure UTs and add more UTs



* Fix failure ITs



* format code



* Fix partial success response not correct issue



* format code



* Fix failure ITs



* Add more UTs to increase code coverage



* Change url regex



* Address comments



* format code



* Fix failure UTs



* Add UT for httpclientFactory throw exception when creating httpclient



* format code



* Address comments and add modelTensor status code



* Address comments



* format code



* Add status code to process error response



* format code



* Rebase main after connector level http parameter support



* Fix UT



* Change error message when remote model return empty and chaange the behavior when one of the requests fails



* Add comments\



* Remove redundant builder and change the error code check



* format code



* Add more UTs for throw exception cases



* fix failure UTs



* format code



* Fix test cases since the error message change



* Rebase code



* fix failure IT



* Add more UTs



* Fix duplicate response to client issue



* fix duplicate response in channel



* change code for all successfully responses case



* Address comments



* format code



* Increase nio httpclient version to fix vulnerbility



* Change validate localhost logic to same with existing code



* change method signature to private



* format code



---------

Signed-off-by: zane-neo <[email protected]>
@mingshl mingshl added the enhancement New feature or request label Apr 30, 2024
dhrubo-os pushed a commit to dhrubo-os/ml-commons that referenced this pull request May 17, 2024
…ect#2375)

* Change httpclient from sync to async



* Change from CRTAsyncHttpClient to NettyAsyncHttpClient



* Add publisher to request



* Change sync httpclient to async



* Handle error case and return error response in actionLListener



* Fix no response when exception



* Add content type header



* Fix issues found in functional test



* Fix no response issue in functional test



* fix default step size error



* Add track inference duration for async httpclient



* Change client appsec highlight issues implementation for async httpclient



* Add UTs



* Add UTs



* Remove unused file



* Add UTs



* format code



* Change error code to honor remote service error code



* Add more UTs



* Change SSRF code to make it correct for return error stattus



* Fix failure UTs and add more UTs



* Fix failure ITs



* format code



* Fix partial success response not correct issue



* format code



* Fix failure ITs



* Add more UTs to increase code coverage



* Change url regex



* Address comments



* format code



* Fix failure UTs



* Add UT for httpclientFactory throw exception when creating httpclient



* format code



* Address comments and add modelTensor status code



* Address comments



* format code



* Add status code to process error response



* format code



* Rebase main after connector level http parameter support



* Fix UT



* Change error message when remote model return empty and chaange the behavior when one of the requests fails



* Add comments\



* Remove redundant builder and change the error code check



* format code



* Add more UTs for throw exception cases



* fix failure UTs



* format code



* Fix test cases since the error message change



* Rebase code



* fix failure IT



* Add more UTs



* Fix duplicate response to client issue



* fix duplicate response in channel



* change code for all successfully responses case



* Address comments



* format code



* Increase nio httpclient version to fix vulnerbility



* Change validate localhost logic to same with existing code



* change method signature to private



* format code



---------

Signed-off-by: zane-neo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants