-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add cpu_weight
config, relates to Systemd's CPUAccounting and CPUWeight
#125
Changes from all commits
e1d39fc
63e152a
9038af9
cba63a8
428259c
a4fcd23
94cfa34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ | |
|
||
from jupyterhub.spawner import Spawner | ||
from jupyterhub.utils import random_port | ||
from traitlets import Bool, Dict, List, Unicode | ||
from traitlets import Bool, Dict, Integer, List, Unicode | ||
|
||
from systemdspawner import systemd | ||
|
||
|
@@ -146,6 +146,18 @@ class SystemdSpawner(Spawner): | |
""", | ||
).tag(config=True) | ||
|
||
cpu_weight = Integer( | ||
None, | ||
allow_none=True, | ||
help=""" | ||
Assign a CPU weight to the single-user notebook server. | ||
Available CPU time is allocated in proportion to each user's weight. | ||
|
||
Acceptable value: an integer between 1 to 10000. | ||
System default is 100. | ||
""", | ||
).tag(config=True) | ||
|
||
def __init__(self, *args, **kwargs): | ||
super().__init__(*args, **kwargs) | ||
# All traitlets configurables are configured by now | ||
|
@@ -276,6 +288,9 @@ async def start(self): | |
else: | ||
working_dir = self._expand_user_vars(self.user_workingdir) | ||
|
||
if self.cpu_weight: | ||
env["CPU_WEIGHT"] = str(self.cpu_weight) | ||
|
||
Comment on lines
+291
to
+293
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To add a new env var here sets a new precendece for systemdspawner of passing environment variables related to the config set. I suspect this can add to maintenance burden long term, so I suggest this is removed unless there is a solid motivation for retaining it. I know some information about system constraints can be detected in a standardized ways, for example as done in dask/distributed. So if this is relevnat information for the user environment, maybe it can be captured in a similar way? |
||
if self.isolate_tmp: | ||
properties["PrivateTmp"] = "yes" | ||
|
||
|
@@ -304,6 +319,11 @@ async def start(self): | |
properties["CPUAccounting"] = "yes" | ||
properties["CPUQuota"] = f"{int(self.cpu_limit * 100)}%" | ||
|
||
if self.cpu_weight is not None: | ||
# FIXME: Detect & use proper properties for v1 vs v2 cgroups | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen similar comments, but I don't know much about it, so I feel quite inactionable about these FIXME comments overall. Are these systemd settings only valid in certain contexts with regards to available cgroup versions? |
||
properties["CPUAccounting"] = "yes" | ||
properties["CPUWeight"] = str(self.cpu_weight) | ||
|
||
if self.disable_user_sudo: | ||
properties["NoNewPrivileges"] = "yes" | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to discussion in another comment about not setting this env var.