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 2, 2020
1 parent dbe2bc4 commit 777edc0
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,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
16 changes: 16 additions & 0 deletions pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"flag"
"fmt"
"io/ioutil"
"math"
"net"
"net/url"
"os"
Expand All @@ -41,6 +42,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 @@ -354,6 +356,20 @@ 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.
if _, set := os.LookupEnv("GOMAXPROCS"); !set {
if cpuInfo, err := cgroups.GetCgroupCPU(); err == nil {
numCPUToUse := int(math.Ceil(cpuInfo.CPUShares()))
if numCPUToUse > runtime.NumCPU() || numCPUToUse <= 0 {
numCPUToUse = runtime.NumCPU()
}
log.Infof(ctx, "running in a container; setting GOMAXPROCS to %d", numCPUToUse)
runtime.GOMAXPROCS(numCPUToUse)
}
}

// 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",
":!cli/start.go",
":!nightly",
":!testutils/lint",
":!util/envutil/env.go",
Expand Down

0 comments on commit 777edc0

Please sign in to comment.