artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 13:11:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat/cmd/gitserver: add memory tracking for both linux and macos (#63114)

Browse Source 
Closes
https://linear.app/sourcegraph/issue/SRC-369/finalize-memory-tracking-in-gitserver-on-linux

This PR adds a memory tracking feature to all gitserver command, using
the utilities introduced in
https://app.graphite.dev/github/pr/sourcegraph/sourcegraph/62803.

When the GITSERVER_MEMORY_OBSERVATION_ENABLED env var is set, an
observer will be spawned that keeps track of the memory usage of the git
invocation. This information is available in traces. Additionally, we'll
print WARN logs if the commands uses more than 100 MB.


## Test plan

Run sg start, and manually tweak the high memory usage threshold down
from 500 MB to 1MB.



```diff
diff --git a/cmd/gitserver/internal/git/gitcli/command.go b/cmd/gitserver/internal/git/gitcli/command.go
index 393fa3c91e6..cf2630fc830 100644
--- a/cmd/gitserver/internal/git/gitcli/command.go
+++ b/cmd/gitserver/internal/git/gitcli/command.go
@@ -297,7 +297,7 @@ func (rc *cmdReader) waitCmd() error {
 	return rc.err
 }
 
-const highMemoryUsageThreshold = 500 * 1024 * 1024 // 500 MiB
+const highMemoryUsageThreshold = 1 * 1024 * 1024 // 500 MiB
 
 func (rc *cmdReader) trace() {
 	duration := time.Since(rc.cmdStart)

```


See log lines similar to the following when I run `sg start` (note the
memory usage information in the log fields):

```
[    gitserver-1] WARN gitserver gitcli/command.go:363 High memory usage exec request {"TraceId": "5881f40e3bbbe4d26ec0f32b7f64f535", "SpanId": "a68818dc9f1f9ee2", "ev.Fields": {"cmd": "config", "cmd_ru_maxrss_kib": "5056", "cmd_ru_maxrss_human_readable": "5.2 MB", "traceID": "5881f40e3bbbe4d26ec0f32b7f64f535", "cmd_ru_inblock": "0", "args": "[git config --unset-all gc.auto]", "actor": "0", "cmd_duration_ms": "6", "user_time_ms": "2", "system_time_ms": "2", "repo": "github.com/sgtest/typescript-deps", "error": "git command [git config --unset-all gc.auto] failed with status code 5 (output: \"\")", "cmd_ru_majflt": "10", "cmd_ru_oublock": "0", "trace": "https://sourcegraph.test:3443/-/debug/jaeger/trace/5881f40e3bbbe4d26ec0f32b7f64f535", "exit_status": "5", "cmd_ru_minflt": "494"}}
[    gitserver-1] WARN gitserver gitcli/command.go:363 High memory usage exec request {"TraceId": "93cb7e9b3a42cfc085f570b8ad4a2ded", "SpanId": "c35ecddb6ef89e24", "ev.Fields": {"cmd_duration_ms": "6", "system_time_ms": "2", "cmd_ru_maxrss_kib": "5072", "cmd_ru_maxrss_human_readable": "5.2 MB", "cmd_ru_minflt": "495", "cmd_ru_majflt": "10", "actor": "0", "exit_status": "5", "user_time_ms": "2", "cmd_ru_oublock": "0", "traceID": "93cb7e9b3a42cfc085f570b8ad4a2ded", "trace": "https://sourcegraph.test:3443/-/debug/jaeger/trace/93cb7e9b3a42cfc085f570b8ad4a2ded", "repo": "github.com/sgtest/update-ids-test-base", "cmd": "config", "args": "[git config --unset-all gc.auto]", "error": "git command [git config --unset-all gc.auto] failed with status code 5 (output: \"\")", "cmd_ru_inblock": "0"}}
[    gitserver-1] WARN gitserver gitcli/command.go:363 High memory usage exec request {"TraceId": "346f1d04dc869f069b04bcabadec0665", "SpanId": "ec43228229e5f531", "ev.Fields": {"actor": "0", "error": "git command [git config --unset-all gc.auto] failed with status code 5 (output: \"\")", "cmd_ru_maxrss_kib": "4960", "trace": "https://sourcegraph.test:3443/-/debug/jaeger/trace/346f1d04dc869f069b04bcabadec0665", "args": "[git config --unset-all gc.auto]", "exit_status": "5", "cmd_duration_ms": "7", "system_time_ms": "2", "repo": "github.com/sgtest/utf8_test", "user_time_ms": "2", "cmd_ru_maxrss_human_readable": "5.1 MB", "cmd_ru_minflt": "487", "cmd_ru_inblock": "0", "cmd_ru_oublock": "0", "traceID": "346f1d04dc869f069b04bcabadec0665", "cmd": "config", "cmd_ru_majflt": "10"}}
[    gitserver-1] WARN gitserver gitcli/command.go:363 High memory usage exec request {"TraceId": "1b6a65835e3f75e7e83c8fe7355baeb2", "SpanId": "d525ac1f8c077184", "ev.Fields": {"cmd_ru_oublock": "0", "repo": "github.com/sourcegraph/about", "args": "[git config --unset-all gc.auto]", "cmd_duration_ms": "7", "system_time_ms": "2", "cmd_ru_inblock": "0", "exit_status": "5", "error": "git command [git config --unset-all gc.auto] failed with status code 5 (output: \"\")", "cmd_ru_maxrss_kib": "4912", "cmd_ru_maxrss_human_readable": "5.0 MB", "cmd_ru_minflt": "483", "cmd": "config", "user_time_ms": "2", "trace": "https://sourcegraph.test:3443/-/debug/jaeger/trace/1b6a65835e3f75e7e83c8fe7355baeb2", "actor": "0", "cmd_ru_majflt": "10", "traceID": "1b6a65835e3f75e7e83c8fe7355baeb2"}}
[    gitserver-1] WARN gitserver gitcli/command.go:363 High memory usage exec request {"TraceId": "39bbc0cc8b99292e6d3b6dfc067d4049", "SpanId": "60e4d69a25a3074b", "ev.Fields": {"args": "[git config --unset-all gc.auto]", "cmd_duration_ms": "7", "cmd_ru_maxrss_kib": "5056", "cmd_ru_minflt": "492", "trace": "https://sourcegraph.test:3443/-/debug/jaeger/trace/39bbc0cc8b99292e6d3b6dfc067d4049", "actor": "0", "system_time_ms": "2", "cmd_ru_inblock": "0", "cmd": "config", "exit_status": "5", "error": "git command [git config --unset-all gc.auto] failed with status code 5 (output: \"\")", "user_time_ms": "2", "cmd_ru_maxrss_human_readable": "5.2 MB", "cmd_ru_oublock": "0", "repo": "github.com/sourcegraph-testing/etcd", "cmd_ru_majflt": "10", "traceID": "39bbc0cc8b99292e6d3b6dfc067d4049"}}
[
```


Note that I have not tested this e2e in a linux system, but I think it's
fine to test it on sourcegraph.com since:

1) we have the benchmarks / integration tests from the previous PR
2) it's behind a environment variable that is off by default

## Changelog 

Adds a new experimental feature to enable track of `git` command memory
invocations when the `GITSERVER_MEMORY_OBSERVATION_ENABLED` environment
variable is true (off by default).
This commit is contained in:
Geoffrey Gilmore 2024-06-10 14:26:41 -07:00 committed by GitHub
parent aa1121c6ba
commit 2c7d217730
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 70 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
cmd/gitserver/internal/git/gitcli/BUILD.bazel
View File

@ -34,13 +34,16 @@ go_library(
"//internal/api",
"//internal/bytesize",
"//internal/byteutils",
"//internal/env",
"//internal/fileutil",
"//internal/gitserver/gitdomain",
"//internal/honey",
"//internal/lazyregexp",
"//internal/memcmd",
"//internal/trace",
"//internal/wrexec",
"//lib/errors",
"@com_github_dustin_go_humanize//:go-humanize",
"@com_github_go_git_go_git_v5//plumbing/format/config",
"@com_github_hashicorp_golang_lru_v2//:golang-lru",
"@com_github_prometheus_client_golang//prometheus",

109
cmd/gitserver/internal/git/gitcli/command.go
View File

@ -7,11 +7,16 @@ import (
"io"
"os"
"os/exec"
"runtime"
"sync"
"syscall"
"time"
"github.com/dustin/go-humanize"
"github.com/sourcegraph/sourcegraph/internal/bytesize"
"github.com/sourcegraph/sourcegraph/internal/env"
"github.com/sourcegraph/sourcegraph/internal/memcmd"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/sourcegraph/log"
@ -20,7 +25,6 @@ import (
"github.com/sourcegraph/sourcegraph/cmd/gitserver/internal/common"
"github.com/sourcegraph/sourcegraph/internal/actor"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/bytesize"
"github.com/sourcegraph/sourcegraph/internal/honey"
"github.com/sourcegraph/sourcegraph/internal/lazyregexp"
"github.com/sourcegraph/sourcegraph/internal/trace"
@ -42,6 +46,8 @@ var (
Name: "src_gitserver_exec_high_memory_usage_count",
Help: "gitcli.GitCommand high memory usage by subcommand",
}, []string{"cmd"})
memoryObservationEnabled = env.MustGetBool("GITSERVER_MEMORY_OBSERVATION_ENABLED", false, "enable memory observation for gitserver commands")
)
type commandOpts struct {
@ -141,7 +147,7 @@ func (g *gitCLIBackend) NewCommand(ctx context.Context, optFns ...CommandOptionF
wrappedCmd := g.rcf.WrapWithRepoName(ctx, logger, g.repoName, cmd)
stdout, err := cmd.StdoutPipe()
stdout, err := wrappedCmd.StdoutPipe()
if err != nil {
cancel()
return nil, errors.Wrap(err, "failed to create stdout pipe")
@ -158,20 +164,38 @@ func (g *gitCLIBackend) NewCommand(ctx context.Context, optFns ...CommandOptionF
return nil, errors.Wrap(err, "failed to start git process")
}
observer := memcmd.NewNoOpObserver()
if memoryObservationEnabled {
maybeObs, err := memcmd.NewDefaultObserver(ctx, cmd)
if err != nil {
logger.Warn("failed to create memory observer, defaulting to no-op", log.Error(err))
}
observer = maybeObs
}
observer.Start()
defer func() {
if err != nil {
observer.Stop() // stop the observer if we return an error
}
}()
execRunning.WithLabelValues(subCmd).Inc()
cr := &cmdReader{
ctx: ctx,
subCmd: subCmd,
ctxCancel: cancel,
cmdStart: cmdStart,
stdout: stdout,
cmd: wrappedCmd,
stderr: stderrBuf,
repoName: g.repoName,
logger: logger,
gitDir: g.dir,
tr: tr,
ctx: ctx,
subCmd: subCmd,
ctxCancel: cancel,
cmdStart: cmdStart,
stdout: stdout,
cmd: wrappedCmd,
stderr: stderrBuf,
repoName: g.repoName,
logger: logger,
gitDir: g.dir,
tr: tr,
memoryObserver: observer,
}
return cr, nil
@ -210,20 +234,21 @@ func (e *commandFailedError) Error() string {
}
type cmdReader struct {
stdout io.Reader
ctx context.Context
ctxCancel context.CancelFunc
subCmd string
cmdStart time.Time
cmd wrexec.Cmder
stderr *bytes.Buffer
logger log.Logger
gitDir common.GitDir
repoName api.RepoName
mu sync.Mutex
tr trace.Trace
err error
waitOnce sync.Once
stdout io.Reader
ctx context.Context
ctxCancel context.CancelFunc
subCmd string
cmdStart time.Time
cmd wrexec.Cmder
stderr *bytes.Buffer
logger log.Logger
gitDir common.GitDir
repoName api.RepoName
mu sync.Mutex
tr trace.Trace
err error
waitOnce sync.Once
memoryObserver memcmd.Observer
}
func (rc *cmdReader) Read(p []byte) (n int, err error) {
@ -255,6 +280,7 @@ func (rc *cmdReader) waitCmd() error {
// here, and memoize the error.
rc.waitOnce.Do(func() {
rc.err = rc.cmd.Wait()
rc.memoryObserver.Stop()
if rc.err != nil {
if checkMaybeCorruptRepo(rc.logger, rc.gitDir, rc.repoName, rc.stderr.String()) {
@ -272,7 +298,7 @@ func (rc *cmdReader) waitCmd() error {
return rc.err
}
// const highMemoryUsageThreshold = 500 * bytesize.MiB
const highMemoryUsageThreshold = 500 * bytesize.MiB
func (rc *cmdReader) trace() {
duration := time.Since(rc.cmdStart)
@ -287,12 +313,14 @@ func (rc *cmdReader) trace() {
sysUsage = *s
}
memUsage := rssToByteSize(sysUsage.Maxrss)
memUsage, err := rc.memoryObserver.MaxMemoryUsage()
if err != nil {
rc.logger.Warn("failed to get max memory usage", log.Error(err))
}
isSlow := duration > shortGitCommandSlow(rc.cmd.Unwrap().Args)
// TODO: Disabled until this also works on linux, this only works on macOS right now
// and causes noise.
isHighMem := false // memUsage > highMemoryUsageThreshold
isHighMem := memUsage > highMemoryUsageThreshold
if isHighMem {
highMemoryCounter.WithLabelValues(rc.subCmd).Inc()
@ -313,7 +341,8 @@ func (rc *cmdReader) trace() {
ev.AddField("cmd_duration_ms", duration.Milliseconds())
ev.AddField("user_time_ms", processState.UserTime().Milliseconds())
ev.AddField("system_time_ms", processState.SystemTime().Milliseconds())
ev.AddField("cmd_ru_maxrss_kib", memUsage/bytesize.KiB)
ev.AddField("cmd_ru_maxrss_kib", memUsage>>10)
ev.AddField("cmd_ru_maxrss_human_readable", humanize.Bytes(uint64(memUsage)))
ev.AddField("cmd_ru_minflt", sysUsage.Minflt)
ev.AddField("cmd_ru_majflt", sysUsage.Majflt)
ev.AddField("cmd_ru_inblock", sysUsage.Inblock)
@ -340,22 +369,14 @@ func (rc *cmdReader) trace() {
rc.tr.SetAttributes(attribute.Int64("cmd_duration_ms", duration.Milliseconds()))
rc.tr.SetAttributes(attribute.Int64("user_time_ms", processState.UserTime().Milliseconds()))
rc.tr.SetAttributes(attribute.Int64("system_time_ms", processState.SystemTime().Milliseconds()))
rc.tr.SetAttributes(attribute.Int64("cmd_ru_maxrss_kib", int64(memUsage/bytesize.KiB)))
rc.tr.SetAttributes(attribute.Int64("cmd_ru_maxrss_kib", int64(memUsage>>10)))
rc.tr.SetAttributes(attribute.String("cmd_ru_maxrss_human_readable", humanize.Bytes(uint64(memUsage))))
rc.tr.SetAttributes(attribute.Int64("cmd_ru_minflt", sysUsage.Minflt))
rc.tr.SetAttributes(attribute.Int64("cmd_ru_majflt", sysUsage.Majflt))
rc.tr.SetAttributes(attribute.Int64("cmd_ru_inblock", sysUsage.Inblock))
rc.tr.SetAttributes(attribute.Int64("cmd_ru_oublock", sysUsage.Oublock))
}
func rssToByteSize(rss int64) bytesize.Bytes {
if runtime.GOOS == "darwin" {
// darwin tracks maxrss in bytes.
return bytesize.Bytes(rss) * bytesize.B
}
// maxrss is tracked in KiB on Linux.
return bytesize.Bytes(rss) * bytesize.KiB
}
const maxStderrCapture = 1024
// stderrBuffer sets up a limited buffer to capture stderr for error reporting.

2
sg.config.yaml
View File

@ -1,6 +1,7 @@
# Documentation for how to override sg configuration for local development:
# https://github.com/sourcegraph/sourcegraph/blob/main/doc/dev/background-information/sg/index.md#configuration
env:
GITSERVER_MEMORY_OBSERVATION_ENABLED: 'true'
PGPORT: 5432
PGHOST: localhost
PGUSER: sourcegraph
@ -1125,6 +1126,7 @@ bazelCommands:
target: //cmd/gitserver
env: &gitserverenv
HOSTNAME: 127.0.0.1:3178
GITSERVER_MEMORY_OBSERVATION_ENABLED: 'true'
# This is only here to stay backwards-compatible with people's custom
# `sg.config.overwrite.yaml` files
gitserver: