search

Boavizta / boaviztapi

🛠 Giving access to BOAVIZTA reference data and methodologies trough a RESTful API
GNU Affero General Public License v3.0
66 stars 21 forks source link

Fix rounding USAGE.instance_per_server #246

Closed JacobValdemar closed 6 months ago

JacobValdemar commented 7 months ago

These roundings doesn't really make sense AFAIK. It is unexpected that e.g. c6g.12xlarge and c6g.16xlarge have the same impact (which was the case before this PR).

PierreRustOrange commented 7 months ago

Hi, just to make sure I get things right , what make you think that the change is related to rounding (which has indeed changed in the last version). Do you have an example ?

thanks

da-ekchajzer commented 7 months ago

@PierreRustOrange, the rounding is related to one data in the inventory. Nothing to do with how the API is handling rounding, no worries.

Sorry @JacobValdemar I just had the time to review the PR. I understand your point. It is a question of how we allocate the impacts of an all machine to one bare metal server. The main issue is that we consider that on machine host on type of EC2 instance, which is not the case (AWS must have an algorithm for optimizing VM distribution).

In the next version, I'd like the allocation to be done for each component and not at machine level, but the problem remains the same.

Pros :

Cons :

@github-benjamin-davy @AirLoren @samuelrince any thought ?

github-benjamin-davy commented 7 months ago

I would need more explanation on the issue and suggested solution but my only comment at this stage would be that in the case of AWS, overcommit is marginal and limited to a few small instances.

JacobValdemar commented 6 months ago

In conclusion, can this be merged?

da-ekchajzer commented 6 months ago

I think https://github.com/Boavizta/boaviztapi/issues/252 will make instance_per_server not needed any more, as allocation will be computed at component level. The issue you highlighted will be fixed with this feature. If ok with you I'll close the PR.