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

Builder pattern for creating control groups #5

Open
levex opened this issue Oct 19, 2018 · 5 comments
Open

Builder pattern for creating control groups #5

levex opened this issue Oct 19, 2018 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@levex
Copy link
Owner

levex commented Oct 19, 2018

I've started working on an API that looks like this:

let cgroup: Cgroup = CgroupBuilder::new("hello", V1)
    .memory()
        .kernel_memory_limit(1024 * 1024)
        .memory_hard_limit(1024 * 1024)
        .done()
    .cpu()
        .shares(100)
        .done()
    .build();

Thoughts?

In particular, I'd prefer to leave out an API that adds a pid to the control group. That's racy and it might introduce subtle race conditions into applications depending on this crate. Instead, the API will likely include an include_command() build that starts the Command in the control group. This would be done via a trait ideally, so people can extend it.

@levex levex self-assigned this Oct 19, 2018
@levex levex added the enhancement New feature or request label Oct 19, 2018
@tecywiz121
Copy link
Collaborator

It does look a lot cleaner! One thing I've noticed about the current API is that if you only set up memory, but you try to add a PID to the Cgroup, it'll attempt to add to all types of cgroup, even the ones you haven't setup yet, which often fails. I think setting it up this way will be more intuitive!

You'll need a function for moving the current process into a cgroup as well.

I'd do something like this for spawning commands:

use std::process::Command;
use std::os::unix::process::CommandExt;

pub trait ComandCgroup {
    fn cgroup(&mut self, &Cgroup) -> &mut Self;
}

impl CommandCgroup for Command {
    fn cgroup(&mut self, cgroup: &Cgroup) -> &mut Self {
        self.before_exec(|| {
            unimplemented!()
        })
    }
}

@levex
Copy link
Owner Author

levex commented Oct 21, 2018

Yeah, lazily creating the controllers is on my TODO list (which I really really should convert into Issues here).

Your suggestion for the trait seems great, I'll add that.

BTW, the current state-of-the-art for the builder pattern is on builder-pattern branch of this repo.

@frol
Copy link

frol commented Dec 6, 2018

@levex I would love to use the new API. Are there any roadblocks? Can I help with sorting them out?

@levex
Copy link
Owner Author

levex commented Dec 7, 2018

Hi @frol, great! No there aren't any. I just need to make a cargo release. I'll make one tomorrow, as I'm unable to do that from this machine.

@frol
Copy link

frol commented Dec 27, 2018

@levex I want to let you know that I ended up implementing my own cgroups-fs crate due to the following issues:

  • I could not attach a child process to the cgroup (you cannot move Cgroup without V1, and I don't actually want to move, but .before_exec captures them).
  • This crate parses/touches too many control files while I only need a few.

Here is what I, essentially, wanted (and built):

let my_cgroup = cgroups_fs::CgroupName::new("my-cgroup");
let my_memory_cgroup = cgroups_fs::AutomanagedCgroup::init(&my_cgroup, "memory").unwrap();

use cgroups_fs::CgroupsCommandExt;
let output = std::process::Command::new("echo")
    .arg("Hello world")
    .cgroups(&[&my_memory_cgroup])
    .output()
    .expect("Failed to execute command");

println!(
    "The echo process used {} bytes of RAM.",
    my_memory_cgroup.get_value::<u64>("memory.max_usage_in_bytes").unwrap()
);

liubin added a commit to liubin/cgroups-rs that referenced this issue Sep 14, 2020
cfs_quota can be set to -1 to indicate no limit

Fixes: levex#5

Signed-off-by: bin liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants