Skip to content

Commit

Permalink
fix(pkg/stash): close etcd sessions
Browse files Browse the repository at this point in the history
Athens creates a new session for each 'Stash' request and then never closes it.
The sessions (etcd leases) are consequently held forever and eventually leads
to resource exhaustion on the etcd cluster.

This has been fixed by closing the acquired session at the end of the request.

Fixes: gomods#1886
  • Loading branch information
uhthomas committed Sep 11, 2023
1 parent e248d22 commit cd0752b
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 23 deletions.
18 changes: 11 additions & 7 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ linters-settings:
cyclop:
max-complexity: 12
skip-tests: true
errcheck:
exclude-functions:
- (*go.etcd.io/etcd/client/v3/concurrency.Mutex).Unlock
- (*go.etcd.io/etcd/client/v3/concurrency.Session).Close
gofumpt:
extra-rules: true

Expand Down Expand Up @@ -50,15 +54,15 @@ linters:
issues:
exclude-use-default: false
exclude:
- 'package-comments: should have a package comment'
- 'ST1000: at least one file in a package should have a package comment'
- 'G204: Subprocess launched with a potential tainted input or cmd arguments'
- 'G204: Subprocess launched with variable'
- 'G402: TLS MinVersion too low.'
- 'const `op` is unused'
- "package-comments: should have a package comment"
- "ST1000: at least one file in a package should have a package comment"
- "G204: Subprocess launched with a potential tainted input or cmd arguments"
- "G204: Subprocess launched with variable"
- "G402: TLS MinVersion too low."
- "const `op` is unused"
exclude-rules:
- path: cmd/proxy/main.go
text: 'G108: Profiling endpoint is automatically exposed on /debug/pprof'
text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
- path: pkg/stash/stasher.go
linters:
- contextcheck
Expand Down
30 changes: 14 additions & 16 deletions pkg/stash/with_etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,33 +56,31 @@ func (s *etcd) Stash(ctx context.Context, mod, ver string) (newVer string, err e
const op errors.Op = "etcd.Stash"
ctx, span := observ.StartSpan(ctx, op.String())
defer span.End()
sesh, err := concurrency.NewSession(s.client)

sess, err := concurrency.NewSession(s.client)
if err != nil {
return ver, errors.E(op, err)
return "", errors.E(op, err)
}
mv := config.FmtModVer(mod, ver)
mu := concurrency.NewMutex(sesh, mv)
err = mu.Lock(ctx)
if err != nil {
return ver, errors.E(op, err)
defer sess.Close()

m := concurrency.NewMutex(sess, config.FmtModVer(mod, ver))
if err := m.Lock(ctx); err != nil {
return "", errors.E(op, err)
}
defer func() {
const op errors.Op = "etcd.Unlock"
lockErr := mu.Unlock(ctx)
if err == nil && lockErr != nil {
err = errors.E(op, lockErr)
}
}()
defer m.Unlock(ctx)

ok, err := s.checker.Exists(ctx, mod, ver)
if err != nil {
return ver, errors.E(op, err)
return "", errors.E(op, err)
}

if ok {
return ver, nil
}

newVer, err = s.stasher.Stash(ctx, mod, ver)
if err != nil {
return ver, errors.E(op, err)
return "", errors.E(op, err)
}
return newVer, nil
}

0 comments on commit cd0752b

Please sign in to comment.