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

[itsbilal]

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.

[cockroach-teamcity]

This change is 

[itsbilal]

@ajwerner I had a conversation with @DuskEagle on how CockroachCloud deals with this currently, and it seems like it doesn't adjust GOMAXPROCS at all. I think this change should still be a no-brainer for the limit-based case, while the docs change in cockroachdb/docs#9001 will be broader advice that'll still apply.

[blathers-crl]

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[ajwerner]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner, @itsbilal, and @knz)

---

pkg/cli/start.go, line 370 at r1 (raw file):

Quoted 6 lines of code…

			if numCPUToUse > runtime.NumCPU() || numCPUToUse <= 0 {
				numCPUToUse = runtime.NumCPU()
			}
			log.Infof(ctx, "running in a container; setting GOMAXPROCS to %d", numCPUToUse)
			runtime.GOMAXPROCS(numCPUToUse)
		}

What about only callong runtime.GOMAXPROCS and logging here if numCPUToUse is sane rather than setting it to runtime.NumCPU() and using that value?

[knz]

Reviewed 3 of 3 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @itsbilal)

---

pkg/testutils/lint/lint_test.go, line 449 at r1 (raw file):

					":!internal/gopath/gopath.go",
					":!cmd",
					":!cli/start.go",

Can you please extract the gomaxproc setting loggic to a different source file, and list that file here instead. We don't want the linter to miss out other stray uses of LookupEnv in start.go.

[itsbilal]

TFTR!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @itsbilal and @knz)

---

pkg/cli/start.go, line 370 at r1 (raw file):

Previously, ajwerner wrote…

			if numCPUToUse > runtime.NumCPU() || numCPUToUse <= 0 {
				numCPUToUse = runtime.NumCPU()
			}
			log.Infof(ctx, "running in a container; setting GOMAXPROCS to %d", numCPUToUse)
			runtime.GOMAXPROCS(numCPUToUse)
		}

What about only callong runtime.GOMAXPROCS and logging here if numCPUToUse is sane rather than setting it to runtime.NumCPU() and using that value?

Done.

---

pkg/testutils/lint/lint_test.go, line 449 at r1 (raw file):

Previously, knz (kena) wrote…

Can you please extract the gomaxproc setting loggic to a different source file, and list that file here instead. We don't want the linter to miss out other stray uses of LookupEnv in start.go.

Done.

[knz]

Reviewed 3 of 3 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained

[itsbilal]

TFTRs!

bors r=knz

[craig]

Build succeeded:

• GitHub CI (Cockroach)