bug(wrap): add oom_score_adj to exec specs#124
Conversation
src/wrap.rs
Outdated
| // If not specified, we do want to pro-actively set this value to the | ||
| // kernel-default of zero, else the subprocess inherits the styrolite | ||
| // oom score (which is typcially set to a very low value). | ||
| fs::write("/proc/self/oom_score_adj", self.exec.oom_score_adj.unwrap_or(0).to_string())?; |
There was a problem hiding this comment.
Might as well set this in AttachRequest::wrap() as well, in roughly the same spot.
There was a problem hiding this comment.
In this case, though, I imagine it would make sense to only set the score if it is specified. In the case of None we would leave it alone, as a running process has already had its oom score set?
In CreateRequest, we are explicit in setting the oom score to zero because the process is fresh, and otherwise linux grabs the value from the parent (which we do not want).
There was a problem hiding this comment.
That's a good point, and on reflection, in both cases, I think we should only set if Some - the force to 0 as a default can be done in the layer above, by the caller, if desired, in that case, and styrolite doesn't have to augur intent around defaults/can be "dumb" about it.
There was a problem hiding this comment.
Which is the layer above here? The zone binary?
In forcing to zero, we would only be restoring the behavior normally you'd expect. We set oom score for zone/styrolite but our process forks to start the app. Linux inherits the score for the child if it was ever set to non-zero by a parent. But us doing so is not intended to trigger this.
I agree keeping styrolite dumb is better, but who else would set this, and early enough, before the process might do it itself?
a65cb4a to
b4551e6
Compare
This value can be forwarded from a CRI and ensures an appropriate OOM score is set for processes in cases where high memory use is seen. Signed-off-by: Alexander Merritt <alexander@edera.dev>
b4551e6 to
5e71d43
Compare
Include consideration of an OOM score adjustment in the process spec. CRI from Kubernetes expects to initialize this depending on how the memory limits are configured. If not specified, we default to the kernel-default of zero.