diff --git a/cmd/gitserver/internal/git/gitcli/BUILD.bazel b/cmd/gitserver/internal/git/gitcli/BUILD.bazel index 71a7dd6c9cd..2cb2e656529 100644 --- a/cmd/gitserver/internal/git/gitcli/BUILD.bazel +++ b/cmd/gitserver/internal/git/gitcli/BUILD.bazel @@ -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", diff --git a/cmd/gitserver/internal/git/gitcli/command.go b/cmd/gitserver/internal/git/gitcli/command.go index 7d7b4065656..bc69b946c9c 100644 --- a/cmd/gitserver/internal/git/gitcli/command.go +++ b/cmd/gitserver/internal/git/gitcli/command.go @@ -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. diff --git a/sg.config.yaml b/sg.config.yaml index 44e1eea2230..68ef31f15fb 100644 --- a/sg.config.yaml +++ b/sg.config.yaml @@ -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: