From 2b2fdffd77630075b29a68cebbebf715541acc35 Mon Sep 17 00:00:00 2001 From: Nikolay Date: Tue, 14 May 2024 09:26:50 +0200 Subject: [PATCH] app/vmauth: explicitly unregister metrics set for auth config (#6252) it's needed to remove Summary metric type from the global state of metrics package. metrics package tracks each bucket of summary and periodically swaps old buckets with new. Simple set unregister is not enough to release memory used by Set https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6247 (cherry picked from commit 6a6e34ab8ec752ae9dfd03e0101625ffb242b9b5) --- app/vmauth/auth_config.go | 8 ++++++++ docs/CHANGELOG.md | 1 + 2 files changed, 9 insertions(+) diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go index e2e38edf0..b4afeee31 100644 --- a/app/vmauth/auth_config.go +++ b/app/vmauth/auth_config.go @@ -693,6 +693,14 @@ func loadAuthConfig() (bool, error) { authConfig.Store(ac) authConfigData.Store(&data) authUsers.Store(&m) + if prevAc != nil { + // explicilty unregister metrics, since all summary type metrics + // are registered at global state of metrics package + // and must be removed from it to release memory. + // Metrics must be unregistered only after atomic.Value.Store calls above + // Otherwise it may lead to metric gaps, since UnregisterAllMetrics is slow operation + prevAc.ms.UnregisterAllMetrics() + } return true, nil } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d14ab38ec..eea50f1b0 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -50,6 +50,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * BUGFIX: [stream aggregation](https://docs.victoriametrics.com/stream-aggregation/): set correct suffix `_prometheus` for aggregation outputs [increase_prometheus](https://docs.victoriametrics.com/stream-aggregation/#increase_prometheus) and [total_prometheus](https://docs.victoriametrics.com/stream-aggregation/#total_prometheus). Before, outputs `total` and `total_prometheus` or `increase` and `increase_prometheus` had the same suffix. * BUGFIX: properly estimate the needed memory for query execution if it has the format [`aggr_func`](https://docs.victoriametrics.com/metricsql/#aggregate-functions)([`rollup_func[d]`](https://docs.victoriametrics.com/metricsql/#rollup-functions) (for example, `sum(rate(request_duration_seconds_bucket[5m]))`). This should allow performing aggregations over bigger number of time series when VictoriaMetrics runs in environments with small amounts of available memory. The issue has been introduced in [this commit](https://github.com/VictoriaMetrics/VictoriaMetrics/commit/5138eaeea0791caa34bcfab410e0ca9cd253cd8f) in [v1.83.0](https://docs.victoriametrics.com/changelog_2022/#v1830). * BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmstorage` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): correctly apply `-inmemoryDataFlushInterval` when it's set to minimum supported value 1s. +* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth/): properly release memory used for metrics during config reload. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6247). * DEPRECATION: [vmagent](https://docs.victoriametrics.com/vmagent/): removed deprecated `-remoteWrite.multitenantURL` flag from vmagent. This flag was deprecated since [v1.96.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.96.0). Use `-enableMultitenantHandlers` instead, as it is easier to use and combine with [multitenant URL at vminsert](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#multitenancy-via-labels). See these [docs for details](https://docs.victoriametrics.com/vmagent.html#multitenancy).