-
Notifications
You must be signed in to change notification settings - Fork 239
add ml_inference processor for offline batch inference #5507
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
Conversation
86e4f34
to
a59fb6a
Compare
.../ml-processor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/MLProcessor.java
Outdated
Show resolved
Hide resolved
...ns/s3-source/src/main/java/org/opensearch/dataprepper/plugins/source/s3/S3ObjectHandler.java
Outdated
Show resolved
Hide resolved
.../ml-processor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/MLProcessor.java
Outdated
Show resolved
Hide resolved
...in/java/org/opensearch/dataprepper/plugins/ml/processor/common/SageMakerBatchJobCreator.java
Outdated
Show resolved
Hide resolved
...in/java/org/opensearch/dataprepper/plugins/ml/processor/common/SageMakerBatchJobCreator.java
Outdated
Show resolved
Hide resolved
...in/java/org/opensearch/dataprepper/plugins/ml/processor/common/SageMakerBatchJobCreator.java
Outdated
Show resolved
Hide resolved
...in/java/org/opensearch/dataprepper/plugins/ml/processor/common/SageMakerBatchJobCreator.java
Outdated
Show resolved
Hide resolved
@Zhangxunmt , Thank you for this great processor! We will also need some unit tests. I'm ok accepting this PR without them as long as we have the |
dlvenable Thanks David for the comments. Looks like there're no major concerns. I will add the remaining UTs soon and the @experimental annotation. |
0b384c9
to
9ce8a77
Compare
5ae7861
to
c670ca5
Compare
d0aa269
to
0108602
Compare
...-processor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/util/RetryUtil.java
Outdated
Show resolved
Hide resolved
...nce-processor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/MLProcessor.java
Outdated
Show resolved
Hide resolved
...ocessor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/MLProcessorConfig.java
Outdated
Show resolved
Hide resolved
...nce-processor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/MLProcessor.java
Outdated
Show resolved
Hide resolved
.../ml-processor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/MLProcessor.java
Outdated
Show resolved
Hide resolved
.../ml-processor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/MLProcessor.java
Outdated
Show resolved
Hide resolved
...nce-processor/src/main/java/org/opensearch/dataprepper/plugins/ml/processor/MLProcessor.java
Outdated
Show resolved
Hide resolved
...in/java/org/opensearch/dataprepper/plugins/ml/processor/common/SageMakerBatchJobCreator.java
Outdated
Show resolved
Hide resolved
@dlvenable Please review the latest commit for the updates to requested changes, after a rebase to the main. The Gradle Builds somehow fail due to unrelated tests. |
...-prepper-plugins/common/src/main/java/org/opensearch/dataprepper/common/utils/RetryUtil.java
Outdated
Show resolved
Hide resolved
return true; // Success | ||
} catch (Exception e) { | ||
try { | ||
Thread.sleep(BASE_DELAY_MS * (1L << attempt)); // Exponential backoff |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will increase quite fast with number of retries. We should use import com.linecorp.armeria.client.retry.Backoff;
and the code should be similar to exponential backoff code in https://github.com/kkondaka/kk-data-prepper-f2/blob/main/data-prepper-plugins/opensearch/src/main/java/org/opensearch/dataprepper/plugins/sink/opensearch/BulkRetryStrategy.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to use similar backoff logic with linecorp.armeria.client
Signed-off-by: Xun Zhang <[email protected]>
Signed-off-by: Xun Zhang <[email protected]>
Signed-off-by: Xun Zhang <[email protected]>
Signed-off-by: Xun Zhang <[email protected]>
…uation Signed-off-by: Xun Zhang <[email protected]>
…put_key is null Signed-off-by: Xun Zhang <[email protected]>
Signed-off-by: Xun Zhang <[email protected]>
Signed-off-by: Xun Zhang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @Zhangxunmt for this contribution!
...n/java/org/opensearch/dataprepper/plugins/ml_inference/processor/client/S3ClientFactory.java
Show resolved
Hide resolved
…oject#5507) Add ml processor for offline batch inference Signed-off-by: Xun Zhang <[email protected]>
…oject#5507) Add ml processor for offline batch inference Signed-off-by: Xun Zhang <[email protected]>
…oject#5507) Add ml processor for offline batch inference Signed-off-by: Xun Zhang <[email protected]>
…oject#5507) Add ml processor for offline batch inference Signed-off-by: Xun Zhang <[email protected]> Signed-off-by: mamol27 <[email protected]>
Description
Adding a new ml_inference processor to interact with ml-commons plugin in OpenSearch for ML related applications.
Some examples that work well:
Issues Resolved
#5470
#5433
#5509
Check List
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.