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: #1886
  • Loading branch information
uhthomas committed Sep 11, 2023
1 parent e248d22 commit ee7ac67
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
17 changes: 10 additions & 7 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ linters-settings:
cyclop:
max-complexity: 12
skip-tests: true
errcheck:
exclude-functions:
- (*go.etcd.io/etcd/client/v3/concurrency).Session
gofumpt:
extra-rules: true

Expand Down Expand Up @@ -50,15 +53,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
7 changes: 5 additions & 2 deletions pkg/stash/with_etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,15 @@ 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)
}
defer sess.Close()

Check failure on line 64 in pkg/stash/with_etcd.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `sess.Close` is not checked (errcheck)

mv := config.FmtModVer(mod, ver)
mu := concurrency.NewMutex(sesh, mv)
mu := concurrency.NewMutex(sess, mv)
err = mu.Lock(ctx)
if err != nil {
return ver, errors.E(op, err)
Expand Down

0 comments on commit ee7ac67

Please sign in to comment.