Skip to content
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

First pass, autosetting within canaryopt 'create' classmethod #424

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

DanielHHowell
Copy link
Contributor

No description provided.

@linear
Copy link

linear bot commented Apr 26, 2022

ENG-622 Auto-set the range of CPU and Memory

Auto-set the range of CPU and memory. Remove the requirement to configure the CPU and memory ranges (min and max), e.g., by using the formula that OKO uses (min = 0.5 * baseline, max = 1.5 * baseline) to derive default values based on the current value of baseline, while still allowing explicit values to be set

@DanielHHowell
Copy link
Contributor Author

@linkous8 can you let me know if this is the right idea, mainly as far as where this logic should take place? And for the actual multipliers (just hardcoded temporarily for testing), do you think they should be constants somewhere or perhaps part of the config?

@DanielHHowell DanielHHowell marked this pull request as ready for review May 11, 2022 20:47
@DanielHHowell DanielHHowell requested a review from linkous8 May 12, 2022 20:10
@DanielHHowell
Copy link
Contributor Author

DanielHHowell commented May 12, 2022

@linkous8 I moved the logic to startup. Any advice on how/where we should store the autosetting multipliers (constants? config? just leave as-is?)
I also added a small touch to just load default ranges when create_tuning_pod is False, doesn't really matter since the values aren't used anyways, just to hold the important pinned settings

Comment on lines +136 to +139
if not self.cpu:
self.cpu = CPU(min="250m", max="3000m")
if not self.memory:
self.memory = Memory(min="256 MiB", max="3.0 GiB")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why have these famous random values versus just raising?

@blakewatters
Copy link
Contributor

startup seems correct here. I'd prolly argue for putting the multipliers into the config with defaults expecting that you never have to change them but you can if you have to. If you hardcode them with constants you need to cut new bits to experiment

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

Successfully merging this pull request may close these issues.

2 participants