Why is it opt-in? Seems like a healthy default. Also, could remove a configuration knob.

There's also implicit configuration in that the system can enable / disable priority management via the capabilities it gives the agent.

Also: aiming to have a review for this by tomorrow afternoon.

I'm confused about why we're processing all processes vs. just the agent PID itself? In theory niceness and oom_score is supposed to be relative—processes that want higher priority should be able to get it without knowledge of every other process.

The current approach whereby we're iterating all processes has scalability issues too. For example, if the agent runs in a large process namespace where it only controls a small number of processes, it would generate immense log spam.

This implementation was adopted from v1 since it is re-implementing functionality requested from a v1 customer. I imagine it was implemented this way because decreasing the niceness score of a process requires CAP_SYS_NICE, whereas increasing it can be done without any additional capabilities. Coder admins are not always sysadmins meaning they may not have the ability to provide additional capabilities.

Why is it opt-in? Seems like a healthy default. Also, could remove a configuration knob.

I figured introducing this sort of functionality is safer to do opt-in, especially because interested parties only need to add a single env var to their template to enable it. If there's any unforeseen issues with the feature we avoid a serious regression by enabling it by default for every customer as opposed to the few who are specifically requesting this feature. Eventually I'd like to promote it to enabled by default once we're confident there aren't any surprises.

I'm confused about why we're processing all processes vs. just the agent PID itself? In theory niceness and oom_score is supposed to be relative—processes that want higher priority should be able to get it without knowledge of every other process.

The current approach whereby we're iterating all processes has scalability issues too. For example, if the agent runs in a large process namespace where it only controls a small number of processes, it would generate immense log spam.

This implementation was adopted from v1 since it is re-implementing functionality requested from a v1 customer. I imagine it was implemented this way because decreasing the niceness score of a process requires CAP_SYS_NICE, whereas increasing it can be done without any additional capabilities. Coder admins are not always sysadmins meaning they may not have the ability to provide additional capabilities.

Why is it opt-in? Seems like a healthy default. Also, could remove a configuration knob.

I figured introducing this sort of functionality is safer to do opt-in, especially because interested parties only need to add a single env var to their template to enable it. If there's any unforeseen issues with the feature we avoid a serious regression by enabling it by default for every customer as opposed to the few who are specifically requesting this feature. Eventually I'd like to promote it to enabled by default once we're confident there aren't any surprises.

SGTM

why not just check if its the current process?

In the future we may want to be able to prioritize other processes such as sshd which is why it's implemented the way it is, but we can just do it pid-based for now.