Skip to content

Commit

Permalink
Fix goroutine leak caused by updating rate quotas (#11371) (#11381)
Browse files Browse the repository at this point in the history
Make sure that when we modify a rate quota, we stop the existing goroutine before starting the new one.
  • Loading branch information
ncabatoff authored Apr 16, 2021
1 parent c312831 commit dfae32a
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 5 deletions.
3 changes: 3 additions & 0 deletions changelog/11371.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
core: Fix goroutine leak when updating rate limit quota
```
6 changes: 4 additions & 2 deletions helper/metricsutil/wrapped_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ func NamespaceLabel(ns *namespace.Namespace) metrics.Label {
case ns.ID == namespace.RootNamespaceID:
return metrics.Label{"namespace", "root"}
default:
return metrics.Label{"namespace",
strings.Trim(ns.Path, "/")}
return metrics.Label{
"namespace",
strings.Trim(ns.Path, "/"),
}
}
}
12 changes: 9 additions & 3 deletions vault/quotas/quotas.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,14 +261,16 @@ func NewManager(logger log.Logger, walkFunc leaseWalkFunc, ms *metricsutil.Clust
return manager, nil
}

// SetQuota adds a new quota rule to the db.
// SetQuota adds or updates a quota rule.
func (m *Manager) SetQuota(ctx context.Context, qType string, quota Quota, loading bool) error {
m.lock.Lock()
defer m.lock.Unlock()
return m.setQuotaLocked(ctx, qType, quota, loading)
}

// setQuotaLocked should be called with the manager's lock held
// setQuotaLocked adds or updates a quota rule, modifying the db as well as
// any runtime elements such as goroutines.
// It should be called with the write lock held.
func (m *Manager) setQuotaLocked(ctx context.Context, qType string, quota Quota, loading bool) error {
if qType == TypeLeaseCount.String() {
m.setIsPerfStandby(quota)
Expand All @@ -277,13 +279,17 @@ func (m *Manager) setQuotaLocked(ctx context.Context, qType string, quota Quota,
txn := m.db.Txn(true)
defer txn.Abort()

raw, err := txn.First(qType, "id", quota.quotaID())
raw, err := txn.First(qType, indexID, quota.quotaID())
if err != nil {
return err
}

// If there already exists an entry in the db, remove that first.
if raw != nil {
quota := raw.(Quota)
if err := quota.close(); err != nil {
return err
}
err = txn.Delete(qType, raw)
if err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions vault/quotas/quotas_rate_limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ func (rlq *RateLimitQuota) allow(req *Request) (Response, error) {
}

// close stops the current running client purge loop.
// It should be called with the write lock held.
func (rlq *RateLimitQuota) close() error {
if rlq.purgeBlocked {
close(rlq.closePurgeBlockedCh)
Expand Down

0 comments on commit dfae32a

Please sign in to comment.