support service profiling manager #71
Conversation
luomingmeng
added
enhancement
luomingmeng
force-pushed
the
dev/support_service_profiling_manager
branch
4 times, most recently
from
fff3a3f to
052e1d4
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
+ Coverage 51.30% 51.46% +0.15%
==========================================
Files 318 334 +16
Lines 32418 33484 +1066
==========================================
+ Hits 16632 17231 +599
- Misses 13840 14219 +379
- Partials 1946 2034 +88
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| f := func(podUID string, containerName string, ci *types.ContainerInfo) bool { | ||
| containerEstimation, err := helper.EstimateContainerMemoryUsage(ci, p.metaReader, p.essentials.EnableReclaim) | ||
| enableReclaim := false | ||
| if p.essentials.EnableReclaim { |
There was a problem hiding this comment.
can we move this logic to util in resource/help.go? (or add a new file in dir resource/) since it is the same for cpu and memory
pkg/metaserver/metaserver.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| var serviceProfilingManager spd.ServiceProfilingManager | ||
| if conf.EnableServiceProfilingManager { |
There was a problem hiding this comment.
why we need a flag to control whether this manager should be enabled or not?
luomingmeng
force-pushed
the
dev/support_service_profiling_manager
branch
5 times, most recently
from
d9779e7 to
9130205
Compare
luomingmeng
force-pushed
the
dev/support_service_profiling_manager
branch
from
9130205 to
3c67f7a
Compare
luomingmeng
added
workflow/need-review
luomingmeng
force-pushed
the
dev/support_service_profiling_manager
branch
from
3c67f7a to
4bfefb0
Compare
pkg/metaserver/spd/manager.go
Outdated
| klog.Infof("[spd-manager] spd %s cache has been deleted", key) | ||
| return nil | ||
| if target.LowerBound != nil && indicatorValue[indicatorName] < *target.LowerBound { | ||
| return true, nil |
There was a problem hiding this comment.
actually, it's hard to say higher or lower indicator value represnts better business performance
| } | ||
|
|
||
| // check whether the pod is degraded | ||
| degraded, err := metaServer.ServiceBusinessPerformanceDegraded(ctx, pod) |
There was a problem hiding this comment.
what about being named as ServiceResourceReclaimEnable? Degrading means a decrease in performance score, but it may not be to indicate that it cannot be co-located with BE pods.
| // avoid frequent requests to the api-server in some bad situations | ||
| s.spdCache.SetLastFetchRemoteTime(key, now) | ||
| for indicatorName, target := range indicatorTarget { | ||
| if target.UpperBound != nil && indicatorValue[indicatorName] > *target.UpperBound { |
There was a problem hiding this comment.
Logic here is questionable: judged as degraded when indicator value > upperbound or indicator value < lowerbound
| return true, nil | ||
| } | ||
|
|
||
| // if pod is degraded, it can not be reclaimed |
There was a problem hiding this comment.
This is not true. Pod cannot be reclaimed when it is degraded over a threshold, not just degraded.
luomingmeng
force-pushed
the
dev/support_service_profiling_manager
branch
from
1900cb0 to
04b7d8e
Compare
luomingmeng
force-pushed
the
dev/support_service_profiling_manager
branch
from
04b7d8e to
4e324d1
Compare
| spdName, err := s.getPodSPDNameFunc(pod) | ||
| func NewServiceProfilingManager(clientSet *client.GenericClientSet, emitter metrics.MetricEmitter, | ||
| cncFetcher cnc.CNCFetcher, conf *pkgconfig.Configuration) (ServiceProfilingManager, error) { | ||
| fetcher, err := NewSPDFetcher(clientSet, emitter, cncFetcher, conf) |
There was a problem hiding this comment.
How about putting spdFetcher into MetaAgent so that we can get spdFetcher to create new ServiceProfilingManager and replace native ServiceProfilingManager with it if needed?
There was a problem hiding this comment.
You can implement custom ServiceProfilingManager by using NewSPDFetcher also, and we doesn't want someone use spd fetcher directly, because spd just one implement of service profiling manager
luomingmeng
added
workflow/merge-ready
What type of PR is this?
Features
What this PR does / why we need it:
We need use a service profiling manager implemented in the meta server to manage service level status for each pod. For example, agents can use it to judge whether this pod can be reclaimed according to this service performance.