From 17878c4c4e69e3c579ab6a01cda9f5c169aa82d4 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Mon, 15 Jul 2024 10:29:56 +0200 Subject: [PATCH] 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 . --- metrics.go | 9 +++++++-- metrics_test.go | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/metrics.go b/metrics.go index 51f236e..74e9735 100644 --- a/metrics.go +++ b/metrics.go @@ -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. diff --git a/metrics_test.go b/metrics_test.go index 7ec4947..7cba9e9 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -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()