Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Add resources to proxy sidecar #51

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

TheEvilCoder42
Copy link

Based on #46 - Add resources to proxy sidecar, hardcoded for now.

@msftclas
Copy link

msftclas commented Oct 25, 2019

CLA assistant check
All CLA requirements met.

@krancour
Copy link
Contributor

Sorry for the delay in responding to this.

The resource requests look a bit higher than what I would have guessed the proxy typically needs (I think the limits look fine).

To be perfectly fair, I don't have hard data on the proxy's resource requirements and haven't bothered to find any because I'm quite certain they're going to vary proportionately to load, but the resource requests being what they are here is going to have a large-ish impact on how many Osiris-enabled pods can be scheduled to a given node. 100m is effectively 10% of a vCPU core and that seems like a lot.

We can lower the requested resources and keep the limits where they are and that won't impact scheduling as much, but will impact the likelihood of pods being evicted from their nodes when resources get tight. (Pods with large discrepancies between resources requested and resource limits are the first to go.) We wouldn't want this either.

I know it was actually my own suggestion to start to solve #46 by hard coding some resource requests and limits, but I am thinking better of that now, considering that our choices are to severely constrain the number of Osiris-enabled workloads per core or else risk evictions. I think we have no reasonable recourse here other than to put this in the hands of the users. i.e. I don't think we can get away with hard coding this after all. You could keep the current numbers as the default, but it would be good to make this tunable via some annotations.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants