-
Notifications
You must be signed in to change notification settings - Fork 152
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
feat: implement container_pid_to_host_pid()
#955
Conversation
This PR needs an update for cgroup v2 support(#865). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good refactoring.
Though, get_conatiner_process_table()
could be reimplemented using /proc/{pid}/status
's NSPid
field (ref: https://man7.org/linux/man-pages/man5/proc.5.html) following get_host_process_table()
, as long as we're on the Linux kernel 4.1 or later, like the existing NSpid
-based code path for converting individual pids. (A notable exception is CentOS 7... and we may keep the current implementation as a fallback for CentOS 7 users and macOS development setups.)
It would be better to avoid spawning subprocesses for this kind of simple queries whenever possible. Even the mapping query itself could be rewritten to just read a single line of I think reading the whole container process table could be also using the procfs /proc/{pid}/status
without contacting the Docker daemon API at all.NSpid
field.
I think we could rewrite get_container_process_table()
as a follow-up PR.
backported-from: main Backported-to: 23.03
This PR implements
ai.backend.agent.utils.container_pid_to_host_pid()
function, which has been left as TODO.