Refactor AutoScaler: metricClient, context, reconcile #308
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kr11
changed the title
Jeffwan
approved these changes
Oct 22, 2024
| return &BaseScalingContext{ | ||
| MaxScaleUpRate: 2, // Scale up rate of 200%, allowing rapid scaling | ||
| MaxScaleDownRate: 2, // Scale down rate of 50%, for more gradual reduction | ||
| ScalingMetric: "CPU", // Metric used for scaling, here set to CPU utilization |
There was a problem hiding this comment.
This should be something has to be configured by user.
There was a problem hiding this comment.
this is OK at this moment, we should add some validation logic later
There was a problem hiding this comment.
Sure, we can allow user to config it by configmap or pa.yaml later.
|
you can rebase the change and get #309 in your code. (CI fix) |
kr11
force-pushed
the
kangrong/features/autoscaling_refactor
branch
from
3592601 to
6621317
Compare
Jeffwan
approved these changes
Oct 22, 2024
Jeffwan
added a commit
that referenced
this pull request
Oct 22, 2024
* Update manifests version to v0.1.0-rc.3 (#287) * [Misc] Add sync images step and scripts in release process (#283) Add sync images step and scripts in release process * [batch] E2E works with driver and request proxy (#272) * e2e driver and test * comment functions * check job status in test * format update * update copyright * add examples with instructions and interfaces * move batch tutorial --------- Co-authored-by: xin.chen <[email protected]> * Fix address already in use when AIRuntime start in pod (#289) add uvicorn startup into file entrypoint * Read model name from request body (#290) * Use model name from request body * rename dummy to reserved router * Fix redis bootstrap flaky connection issue (#293) * skip docs CI if no changes in /docs dir (#294) * skip docs CI if no changes in /docs dir * test docs build * Improve Rayclusterreplicaset Status (#295) * improve rayclusterreplicaset status * nit * fix lint error * improve isClusterActive logic * fix lint error * remove redundant isRayPodCreateOrDeleteFailed check --------- Signed-off-by: Yicheng-Lu-llll <[email protected]> * Add request trace for profiling (#291) * Add request trace for profiling * add to redis at 10 second interval * nit * round to nearest 10s interval * round timestamp to nearest 10s interval and aggregate data by model * add go routine to add request trace * Update the crd definiton due to runtime upgrade (#298) #295 introduce the latest kuberay api and the dependencies bumps sigs.k8s.io/controller-runtime from v0.17.3 to v0.17.5. Due to that change, make manifest update the CRD definitions * Push images to Github registry in release pipeline (#301) * Disable docker build github workflow to cut CI cost * Push images to Github registry in release pipeline * Build autoscaler abstractions like fetcher, client and scaler (#300) * minor clean up on the autoscaler controller * Extract the algorithm package algorithm is extracted to distinguish with the scaler. * Refactor scaler interface 1. Split the Scaler interface and BaseAutoscaler implementation 2. Create APA/KPA scaler separately and adopt the corresponding algorithms * Introduce the scalingContext in algorithm * Introduce k8s.io/metrics for resource & custom metrics fetching * Extract metric fetcher to cover the fetching logic * Optimize the scaler workflow to adopt fetch and client interface * Further refactor the code structure * Support pod autoscaler periodically check (#306) * Support pod autoscaler periodically check * Fix the error case * Add timeout in nc check for redis bootstrap (#309) * Refactor AutoScaler: metricClient, context, reconcile (#308) * Refactor AutoScaler: optimize metric client, context, and reconcile processes. * fix make lint-all * fix typos --------- Signed-off-by: Yicheng-Lu-llll <[email protected]> Co-authored-by: xinchen384 <[email protected]> Co-authored-by: xin.chen <[email protected]> Co-authored-by: brosoul <[email protected]> Co-authored-by: Varun Gupta <[email protected]> Co-authored-by: Yicheng-Lu-llll <[email protected]> Co-authored-by: Rong-Kang <[email protected]>
gangmuk
pushed a commit
that referenced
this pull request
Jan 25, 2025
* Refactor AutoScaler: optimize metric client, context, and reconcile processes. * fix make lint-all * fix typos
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Based on the suggestions in #119 comments, we continue refactoring of the autoscaling code structure, which have done:
reconcileKPAandreconcileAPAintoreconcileCustomPA, thereby sharing common stages and adapting different PA-specific components based on thepa.ScalingStrategy.TBD 4.
metricName->metricKeyCurrently, the
metricName stringis utilized inGetPodContainerMetricandGetMetricsFromPods. Both functions also includepods []corev1.Podas parameters, which already encompass themetric_keysinformation. Should we continue to usemetricNamein this context?TBD 5. ScalingContext - Change from Interface to Struct:
I have transferred some common attributes from
KpaScalingContexttoBaseScalingContextto make them concise.I recommend retaining the
ScalingContextinterface, as theScalingAlgorithm.ComputeTargetReplicas, a higher-level interface that operates above specific algorithms, requiresScalingContextas a parameter.Related Issues
Resolves: #[119]
Related issue list in comments.
Important: Before submitting, please complete the description above and review the checklist below.
Contribution Guidelines (Expand for Details)
We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:
Pull Request Title Format
Your PR title should start with one of these prefixes to indicate the nature of the change:
[Bug]: Corrections to existing functionality[CI]: Changes to build process or CI pipeline[Docs]: Updates or additions to documentation[API]: Modifications to aibrix's API or interface[CLI]: Changes or additions to the Command Line Interface[Misc]: For changes not covered above (use sparingly)Note: For changes spanning multiple categories, use multiple prefixes in order of importance.
Submission Checklist
By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.