-
Notifications
You must be signed in to change notification settings - Fork 261
runtime, envelope: add support for cgroup
for child
processes.
#770
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
Conversation
@rgrandl @mwhittaker While working on one of my project I needed to spawn weavelets under fixed CPU and memory constraints so I added support for that in my private fork, I think this will be a handy feature for upstream. Do you have any opinions on this one ? Ofcourse I need to harden this PR a bit more so cgroup specs can be accepted from |
@flouthoc , very interesting feature. Will discuss this in our weekly meeting and will come back to you. |
@rgrandl just a gentle bump to see if you were able to get time to discuss this one :) |
@flouthoc, we've discussed this. We were thinking of an alternate solution: would it be possible to create these cgroups beforehand, and then when you start the child process, specify the name of the cgroup(s) under which the process should run? |
@rgrandl Yes sounds reasonable to me, so service-weaver should only contain the part of moving processes to the cgroup, is that correct and user should specify Also Just curious any specific reason about why |
Allow envelope to accept cgroup manager which gives users oppertunity to constraint process created by weavelets, which is really helpful in case of `multi` and `single` deployment where user would want to limit `cpu`, `memory` usage or other resource usage. Signed-off-by: flouthoc <[email protected]>
Allow loading cgroups from config and create envelope using `cgroup` if they are configured. Signed-off-by: flouthoc <[email protected]>
Run `./dev/build_and_test tidy generate build vet test testrace` on source Signed-off-by: flouthoc <[email protected]>
@rgrandl Done, added change so now it can accept externally created Let me know if config fields look fine then I can also add this feature to |
@rgrandl Any thoughts on this ? |
Gentle bump on the PR. |
Wondering if anything is pending on this PR from my side. I am just waiting for another review round on this one. |
@flouthoc , sorry for being quiet on this. I've been distracted with some other things. I've been waiting to discuss your change with some other team members, because the design is a little bit intrusive/tricky, but haven't had a chance to do that yet. Is this a blocker for you? I will come back with comments on this soon. |
@rgrandl No not a blocker at all. This is something which I had in my fork just thought it'd be a nice patch for upstream. Please take your time :) |
This is something we've never managed to agree on as a team. Close it for now. |
Sure no issues :) |
Allow envelope to accept cgroup manager which gives users opportunity to
constraint process created by weavelets, which is really helpful in case
of
multi
andsingle
deployment where user would want to limitcpu
,memory
usage or other resource usage.multi: create and provide cgroup manager