Skip to content

Commit

Permalink
cli: Set GOMAXPROCS on start if in a CPU-limited cgroup
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
itsbilal committed Dec 4, 2020
1 parent 1421d00 commit 9dfca6b
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 1 deletion.
1 change: 1 addition & 0 deletions pkg/cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ go_library(
"//pkg/storage/enginepb",
"//pkg/ts/tspb",
"//pkg/util",
"//pkg/util/cgroups",
"//pkg/util/contextutil",
"//pkg/util/encoding",
"//pkg/util/encoding/csv",
Expand Down
6 changes: 6 additions & 0 deletions pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/util/cgroups"
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/cockroach/pkg/util/grpcutil"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
Expand Down Expand Up @@ -359,6 +360,11 @@ func runStart(cmd *cobra.Command, args []string, startSingleNode bool) (returnEr
// but when actually starting a server, we enable them.
grpcutil.LowerSeverity(severity.WARNING)

// Tweak GOMAXPROCS if we're in a cgroup / container that has cpu limits set.
// The GO default for GOMAXPROCS is runtime.NumCPU(), however this is less
// than ideal if the cgruop is limited to a number lower than that.
cgroups.AdjustMaxProcs(ctx)

// Check the --join flag.
if !flagSetForCmd(cmd).Lookup(cliflags.Join.Name).Changed {
err := errors.WithHint(
Expand Down
1 change: 1 addition & 0 deletions pkg/testutils/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ func TestLint(t *testing.T) {
":!ccl/workloadccl/fixture_test.go",
":!internal/gopath/gopath.go",
":!cmd",
":!util/cgroups/cgroups.go",
":!nightly",
":!testutils/lint",
":!util/envutil/env.go",
Expand Down
5 changes: 4 additions & 1 deletion pkg/util/cgroups/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ go_library(
srcs = ["cgroups.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/util/cgroups",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/cockroachdb/errors"],
deps = [
"//pkg/util/log",
"//vendor/github.com/cockroachdb/errors",
],
)

go_test(
Expand Down
19 changes: 19 additions & 0 deletions pkg/util/cgroups/cgroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package cgroups
import (
"bufio"
"bytes"
"context"
"fmt"
"io/ioutil"
"math"
Expand All @@ -22,6 +23,7 @@ import (
"strconv"
"strings"

"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
)

Expand Down Expand Up @@ -444,3 +446,20 @@ func getCgroupCPU(root string) (CPUUsage, error) {

return res, nil
}

// AdjustMaxProcs sets GOMAXPROCS (if not overridden by env variables) to be
// the CPU limit of the current cgroup, if running inside a cgroup with a cpu
// limit lower than runtime.NumCPU(). This is preferable to letting it fall back
// to Go default, which is runtime.NumCPU(), as the Go scheduler would be
// running more OS-level threads than can ever be concurrently scheduled.
func AdjustMaxProcs(ctx context.Context) {
if _, set := os.LookupEnv("GOMAXPROCS"); !set {
if cpuInfo, err := GetCgroupCPU(); err == nil {
numCPUToUse := int(math.Ceil(cpuInfo.CPUShares()))
if numCPUToUse < runtime.NumCPU() && numCPUToUse > 0 {
log.Infof(ctx, "running in a container; setting GOMAXPROCS to %d", numCPUToUse)
runtime.GOMAXPROCS(numCPUToUse)
}
}
}
}

0 comments on commit 9dfca6b

Please sign in to comment.