Skip to content

Commit

Permalink
Make UnregisterSet() less error-prone to use
Browse files Browse the repository at this point in the history
Previously it was expected that the user calls s.UnregisterAllMetrics() after calling UnregisterSet(s).
This led to many subtle memory leak bugs like VictoriaMetrics/VictoriaMetrics#6247 .

Solve this issue by adding `destroySet bool` arg to UnregisterSet(), so it automatically calls
s.UnregisterAllMetrics() if destroySet=true.

This changes UnregisterSet() function signature, so users need to update UnregisterSet() calls across their code bases
after upgrading to the new version of github.com/VictoriaMetrics/metrics package. This is OK, since this allows
fixing subtle memory leak bugs like VictoriaMetrics/VictoriaMetrics#6247 .
  • Loading branch information
valyala committed Jul 15, 2024
1 parent 32321d6 commit 17878c4
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
9 changes: 7 additions & 2 deletions metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,16 @@ func RegisterSet(s *Set) {

// UnregisterSet stops exporting metrics for the given s via global WritePrometheus() call.
//
// Call s.UnregisterAllMetrics() after unregistering s if it is no longer used.
func UnregisterSet(s *Set) {
// If destroySet is set to true, then s.UnregisterAllMetrics() is called on s after unregistering it,
// so s becomes destroyed. Otherwise the s can be registered again in the set by passing it to RegisterSet().
func UnregisterSet(s *Set, destroySet bool) {
registeredSetsLock.Lock()
delete(registeredSets, s)
registeredSetsLock.Unlock()

if destroySet {
s.UnregisterAllMetrics()
}
}

// RegisterMetricsWriter registers writeMetrics callback for including metrics in the output generated by WritePrometheus.
Expand Down
2 changes: 1 addition & 1 deletion metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestRegisterUnregisterSet(t *testing.T) {
t.Fatalf("missing %q in\n%s", expectedLine, data)
}

UnregisterSet(s)
UnregisterSet(s, true)
bb.Reset()
WritePrometheus(&bb, false)
data = bb.String()
Expand Down

0 comments on commit 17878c4

Please sign in to comment.