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.

Sign up for GitHub

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

Jump to bottom

executor: stop joining executor to container cgroup #6839

Merged
merged 3 commits into from
Dec 13, 2019
Merged

executor: stop joining executor to container cgroup #6839

merged 3 commits into from
Dec 13, 2019

Commits on Dec 11, 2019

  1. drivers/exec: test all cgroups are destroyed

    Mahmood Ali committed Dec 11, 2019
    Configuration menu
    Copy the full SHA
    2f4b9da View commit details
    Browse the repository at this point in the history

  2. executor: stop joining executor to container cgroup 

    Stop joining libcontainer executor process into the newly created task
container cgroup, to ensure that the cgroups are fully destroyed on
shutdown, and to make it consistent with other plugin processes.

Previously, executor process is added to the container cgroup so the
executor process resources get aggregated along with user processes in
our metric aggregation.

However, adding executor process to container cgroup adds some
complications with much benefits:

First, it complicates cleanup.  We must ensure that the executor is
removed from container cgroup on shutdown.  Though, we had a bug where
we missed removing it from the systemd cgroup.  Because executor uses
`containerState.CgroupPaths` on launch, which includes systemd, but
`cgroups.GetAllSubsystems` which doesn't.

Second, it may have advese side-effects.  When a user process is cpu
bound or uses too much memory, executor should remain functioning
without risk of being killed (by OOM killer) or throttled.

Third, it is inconsistent with other drivers and plugins.  Logmon and
DockerLogger processes aren't in the task cgroups.  Neither are
containerd processes, though it is equivalent to executor in
responsibility.

Fourth, in my experience when executor process moves cgroup while it's
running, the cgroup aggregation is odd.  The cgroup
`memory.usage_in_bytes` doesn't seem to capture the full memory usage of
the executor process and becomes a red-harring when investigating memory
issues.

For all the reasons above, I opted to have executor remain in nomad
agent cgroup and we can revisit this when we have a better story for
plugin process cgroup management.
    Mahmood Ali committed Dec 11, 2019
    Configuration menu
    Copy the full SHA
    596d0be View commit details
    Browse the repository at this point in the history

  3. simplify cgroup path lookup

    Mahmood Ali committed Dec 11, 2019
    Configuration menu
    Copy the full SHA
    f794b49 View commit details
    Browse the repository at this point in the history