Make UnregisterSet() less error-prone to use

Previously it was expected that the user calls s.UnregisterAllMetrics() after calling UnregisterSet(s).
This led to many subtle memory leak bugs like https://github.com/VictoriaMetrics/VictoriaMetrics/issues/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 https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6247 .
This commit is contained in:
Aliaksandr Valialkin 2024-07-15 10:29:56 +02:00
parent 32321d68e1
commit 17878c4c4e
No known key found for this signature in database
GPG Key ID: 52C003EE2BCDB9EB
2 changed files with 8 additions and 3 deletions

View File

@ -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.

View File

@ -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()