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

Add cpu_weight config, relates to Systemd's CPUAccounting and CPUWeight #125

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ in your `jupyterhub_config.py` file:

- **[`mem_limit`](#mem_limit)**
- **[`cpu_limit`](#cpu_limit)**
- **[`cpu_weight`](#cpu_weight)**
- **[`user_workingdir`](#user_workingdir)**
- **[`username_template`](#username_template)**
- **[`default_shell`](#default_shell)**
Expand Down Expand Up @@ -214,6 +215,23 @@ This works out perfect for most cases, since this allows users to burst up and
use all CPU when nobody else is using CPU & forces them to automatically yield
when other users want to use the CPU.

The share of access each user gets can be adjusted with the `cpu_weight` option.

### `cpu_weight`

An integer representing the share of CPU time each user can use. A user with
a `cpu_weight` of 200 will get 2x access to the CPU than a user with a `cpu_weight`
of 100, which is the system default.

```python
c.SystemdSpawner.cpu_weight = 100
```

Defaults to `None`, which implicitly means 100.

This info is exposed to the single-user server as the environment variable
`CPU_WEIGHT` as an integer.

Comment on lines +232 to +234
Copy link
Member

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.

### `user_workingdir`

The directory to spawn each user's notebook server in. This directory is what users
Expand Down
22 changes: 21 additions & 1 deletion systemdspawner/systemdspawner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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"

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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"

Expand Down