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

release-20.2: backport cgroup related changes to improve CPU and memory accounting #59184

Merged
merged 3 commits into from
Jan 20, 2021
Merged

release-20.2: backport cgroup related changes to improve CPU and memory accounting #59184

merged 3 commits into from
Jan 20, 2021

Commits on Jan 20, 2021

  1. util, server: Add GetCgroupCPU, use it to reflect cpu usage 

    Enhances the util/cgroups package to also support getting
CPU limits from the process' current cgroup. The cpu limit / share
for the current cgroup is denoted in two separate variables:
the cpu period, and the cpu quota. When (quota / period) = numCPUs,
this cgroup has access to all the CPUs on the system.

This PR also updates SampleEnvironment to call this method
and adjust the cpu usage % accordingly.

Release note (bug fix): Improve accuracy of reported CPU usage
when running in containers.
    @itsbilal @darinpp
    itsbilal authored and darinpp committed Jan 20, 2021
    Configuration menu
    Copy the full SHA
    41412b4 View commit details
    Browse the repository at this point in the history

  2. util, server: fix memory detection with non-root cgroups 

    The cgroup information is constructed by looking at `/proc/self/cgroup`,
matching with the mount point in `/proc/self/mountinfo`
and then inpsecting the `memory.stat` file. The matching between the
cgroup and the mount point was working only in case that the mount and
the cgroup are exact match relative to cgroup NS. This is however
inadequate as while true for the docker case, it isn't true in the general
case. In this example determining the cgroup memory limit isn't possible:
```
cgcreate -t $USER:$USER -a $USER:$USER -g memory:crdb_test
echo 3999997952 > /sys/fs/cgroup/memory/crdb_test/memory.limit_in_bytes
cgexec -g memory:crdb_test ./cockroach start-single-nod
```
To address this, this patch instead constructs the expected
relative path of the cgroup's memory info by starting with the
mount path and then adding the cgroup.

Release note: None
    @darinpp
    darinpp committed Jan 20, 2021
    Configuration menu
    Copy the full SHA
    b4a91f5 View commit details
    Browse the repository at this point in the history

  3. cli: Set GOMAXPROCS on start if in a CPU-limited cgroup 

    Having a GOMAXPROCS value defaulting to the Go default
(numCPUs) is inefficient if there's a lower CPU limit
set for the cgroup the cockroach process is in. The
Go runtime could be scheduling multiple OS-level
threads, not all of which would get to run concurrently.

This change sees if the GOMAXPROCS env variable
was set, denoting an overriden value. If it isn't
set, and we're inside a cgroup, the value of
GOMAXPROCS is now lowered to the maximum

This is an implementation of part of the advice given
in cockroachdb/docs#9001 .
The CPU requests/shares part of that equation cannot
be implemented in Cockroach, so the docs issue remains
unchanged.

Release note (general change): Run fewer threads in parallel if
running inside a container with a CPU limit.
    @itsbilal @darinpp
    itsbilal authored and darinpp committed Jan 20, 2021
    Configuration menu
    Copy the full SHA
    87d3c5f View commit details
    Browse the repository at this point in the history