-
Notifications
You must be signed in to change notification settings - Fork 385
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 targetUtilization for percentile prediction #330
Conversation
2. Refactor percentile prediction
🎉 Successfully Build Images. Overview: https://finops.coding.net/public-artifacts/gocrane/crane/packages
|
SampleInterval: sampleInterval, | ||
MarginFraction: marginFraction, | ||
TargetUtilization: targetUtilization, | ||
Percentile: percentile, |
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.
Please make sure the go crane api is updated. now our structure is hard to extend, so maybe later we change the algorithm params to just a map to avoid the frequently updated to the crane api
@@ -226,40 +193,7 @@ func (p *percentilePrediction) process(namer metricnaming.MetricNamer, config co | |||
} | |||
} | |||
|
|||
estimator := NewPercentileEstimator(cfg.percentile) |
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.
Why remove this? this is used to do once task. it do not depends on the state signal cached in memory
@@ -57,6 +58,46 @@ func generateSamplesFromWindow(value float64, start time.Time, end time.Time, st | |||
return result | |||
} | |||
|
|||
func (p *percentilePrediction) getPredictedValuesFromSignals(queryExpr string, signals map[string]*aggregateSignal) []*common.TimeSeries { |
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.
we need abstract a common lib for algorithm functions
|
||
targetUtilization, exists := props["resource.mem-target-utilization"] | ||
if !exists { | ||
targetUtilization = "1.0" |
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.
replicas.min-cpu-target-utilization: "30" replicas.max-cpu-target-utilization: "75" replicas.cpu-target-utilization: "50"
currently xxx-utilization is a 100 percent based float, better to align the same format for new configuration.
also please add it in deploy/craned/deployment.yaml and also the helm chart.
|
1. Add targetUtilization for percentile prediction 2. Refactor percentile prediction 3. Fix analytics reconcile loop issue Co-authored-by: yuyufei <yufeiyu@tencent.com>
What type of PR is this?
feat
What this PR does / why we need it:
Add targetUtilization for percentile prediction
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: