search

kruize / autotune

Autonomous Performance Tuning for Kubernetes!
Apache License 2.0
157 stars 53 forks source link

Make CPU Limits to point correctly, earlier it's pointing to cpu request #1023

Closed bharathappali closed 10 months ago

bharathappali commented 10 months ago

As mentioned in issue #1022 the variation is not matching the difference between current and recommended value. The issue is due to the incorrect mapping of cpu limits to cpu requests.

@dinogun Can I please have your review?

bharathappali commented 10 months ago

Thanks @khansaad for pointing the issue

dinogun commented 10 months ago

Good catch indeed @khansaad!!

bharathappali commented 10 months ago

@dinogun It is jus calculating a difference so I have added it in the engines, Can be extracted and common out.

bharathappali commented 10 months ago

Do we need to do this calculation in both the engines?

Yes, calculations are done in both the engines.

Can this be done outside in common code

Yes can be done, but after a detailed inspection I felt it would be better to have these calculations done in the respective engines as these calculations are tightly coupled to populating the notifications and also lot many objects need to be sent to the common code to take decisions and calculate the differences, Moreover these engines using the data for representational purpose as of now, but in future if engine needs to take some actions or need to tweak certain things in the data it would be hard to do it in common code as we need to pass the engine name and implement engine specific behaviour over there which might be not correct considering the abstractions built around engines to do the engine specific work in their scope. Please lemme know your thoughts on this.

dinogun commented 10 months ago

ok, we can schedule the refactoring for a later time then