-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add per-tenant provisioning throttling #1074
Add per-tenant provisioning throttling #1074
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (69.01%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1074 +/- ##
============================================
+ Coverage 75.57% 76.32% +0.75%
- Complexity 1050 1075 +25
============================================
Files 101 101
Lines 5212 5276 +64
Branches 498 503 +5
============================================
+ Hits 3939 4027 +88
+ Misses 1035 1001 -34
- Partials 238 248 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4c7e7f6
to
fb23bea
Compare
fb23bea
to
42ad240
Compare
42ad240
to
cae1242
Compare
tests failing because of jar hell caused by opensearch-project/ml-commons#3475 cc @owaiskazi19 |
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
cae1242
to
b4a6fd9
Compare
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.
Looks good overall with comments
src/main/java/org/opensearch/flowframework/util/TenantAwareHelper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/util/TenantAwareHelper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/common/FlowFrameworkSettings.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowTransportAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/util/TenantAwareHelper.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/flowframework/rest/RestWorkflowProvisionTenantAwareIT.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/common/FlowFrameworkSettings.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/common/FlowFrameworkSettings.java
Show resolved
Hide resolved
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@owaiskazi19 @amitgalitz Thanks for your review! I've addressed some comments in 6fbf718:
I'd like to handle changing the value of settings/limits and defaults in another PR (followed by a discussion issue). I'll consider refactoring all of the multitenant ITs at a later date. Other comments I've responded that I don't think changes are needed; I've added some comments in code explaining why. In particular I don't think there's a easy way to replace the increment/rollback logic with compareAndSet without really overcomplicating things. |
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.
Thanks for addressing the comments
Description
Adds settings to control per-tenant limits on simultaneous provision, reprovision, and deprovision.
Check List
[ ] API changes companion pull request created.--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.