From 2a7207f38ace1a40e87330508cec9b4e6e26a59b Mon Sep 17 00:00:00 2001
From: hagen1778 <roman@victoriametrics.com>
Date: Tue, 9 Jan 2024 13:17:09 +0100
Subject: [PATCH] app/all: follow-up after
https://github.com/VictoriaMetrics/VictoriaMetrics/commit/84d710beab98067e88badbe4af4e0b523e2f6076
https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5548
Signed-off-by: hagen1778 <roman@victoriametrics.com>
---
app/victoria-logs/main.go | 2 ++
app/vmagent/main.go | 1 +
app/vmalert/main.go | 1 +
app/vmauth/main.go | 1 +
app/vmbackup/main.go | 1 +
app/vminsert/main.go | 2 ++
app/vmrestore/main.go | 1 +
app/vmselect/main.go | 2 ++
app/vmstorage/main.go | 4 +---
docs/CHANGELOG.md | 1 +
lib/pushmetrics/pushmetrics.go | 14 +++++++++-----
lib/uint64set/uint64set_test.go | 34 +++++++++++++++++++++++++++++++++
12 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/app/victoria-logs/main.go b/app/victoria-logs/main.go
index 5cf3add9a8..62d952c07a 100644
--- a/app/victoria-logs/main.go
+++ b/app/victoria-logs/main.go
@@ -59,6 +59,8 @@ func main() {
}
logger.Infof("successfully shut down the webservice in %.3f seconds", time.Since(startTime).Seconds())
+ pushmetrics.Stop()
+
vlinsert.Stop()
vlselect.Stop()
vlstorage.Stop()
diff --git a/app/vmagent/main.go b/app/vmagent/main.go
index 9f66d54e8e..c681a84fae 100644
--- a/app/vmagent/main.go
+++ b/app/vmagent/main.go
@@ -159,6 +159,7 @@ func main() {
logger.Infof("successfully shut down the webservice in %.3f seconds", time.Since(startTime).Seconds())
}
+ pushmetrics.Stop()
promscrape.Stop()
if len(*influxListenAddr) > 0 {
diff --git a/app/vmalert/main.go b/app/vmalert/main.go
index a87897204c..be8f0c1bde 100644
--- a/app/vmalert/main.go
+++ b/app/vmalert/main.go
@@ -187,6 +187,7 @@ func main() {
if err := httpserver.Stop(*httpListenAddr); err != nil {
logger.Fatalf("cannot stop the webservice: %s", err)
}
+ pushmetrics.Stop()
cancel()
manager.close()
}
diff --git a/app/vmauth/main.go b/app/vmauth/main.go
index b206619774..df9eeda007 100644
--- a/app/vmauth/main.go
+++ b/app/vmauth/main.go
@@ -80,6 +80,7 @@ func main() {
if err := httpserver.Stop(*httpListenAddr); err != nil {
logger.Fatalf("cannot stop the webservice: %s", err)
}
+ pushmetrics.Stop()
logger.Infof("successfully shut down the webservice in %.3f seconds", time.Since(startTime).Seconds())
stopAuthConfig()
logger.Infof("successfully stopped vmauth in %.3f seconds", time.Since(startTime).Seconds())
diff --git a/app/vmbackup/main.go b/app/vmbackup/main.go
index 40c5c8bf43..2ce83d476d 100644
--- a/app/vmbackup/main.go
+++ b/app/vmbackup/main.go
@@ -107,6 +107,7 @@ func main() {
if err := httpserver.Stop(*httpListenAddr); err != nil {
logger.Fatalf("cannot stop http server for metrics: %s", err)
}
+ pushmetrics.Stop()
logger.Infof("successfully shut down http server for metrics in %.3f seconds", time.Since(startTime).Seconds())
}
diff --git a/app/vminsert/main.go b/app/vminsert/main.go
index 6d6c4438c0..9c23117a51 100644
--- a/app/vminsert/main.go
+++ b/app/vminsert/main.go
@@ -161,6 +161,8 @@ func main() {
}
logger.Infof("successfully shut down http service in %.3f seconds", time.Since(startTime).Seconds())
+ pushmetrics.Stop()
+
if len(*clusternativeListenAddr) > 0 {
clusternativeServer.MustStop()
}
diff --git a/app/vmrestore/main.go b/app/vmrestore/main.go
index cb2efcd79d..799d56ba7e 100644
--- a/app/vmrestore/main.go
+++ b/app/vmrestore/main.go
@@ -65,6 +65,7 @@ func main() {
if err := httpserver.Stop(*httpListenAddr); err != nil {
logger.Fatalf("cannot stop http server for metrics: %s", err)
}
+ pushmetrics.Stop()
logger.Infof("successfully shut down http server for metrics in %.3f seconds", time.Since(startTime).Seconds())
}
diff --git a/app/vmselect/main.go b/app/vmselect/main.go
index 52810d9b1a..268a45e62c 100644
--- a/app/vmselect/main.go
+++ b/app/vmselect/main.go
@@ -145,6 +145,8 @@ func main() {
}
logger.Infof("successfully shut down http service in %.3f seconds", time.Since(startTime).Seconds())
+ pushmetrics.Stop()
+
if vmselectapiServer != nil {
logger.Infof("stopping vmselectapi server...")
vmselectapiServer.MustStop()
diff --git a/app/vmstorage/main.go b/app/vmstorage/main.go
index 8f9b0f9f5b..bf8c9cb90d 100644
--- a/app/vmstorage/main.go
+++ b/app/vmstorage/main.go
@@ -144,9 +144,7 @@ func main() {
}
logger.Infof("successfully shut down http service in %.3f seconds", time.Since(startTime).Seconds())
- // close the metric reporting goroutine
- pushmetrics.StopPushMetrics()
- logger.Infof("successfully stop push metric in %.3f seconds", time.Since(startTime).Seconds())
+ pushmetrics.Stop()
logger.Infof("gracefully shutting down the service")
startTime = time.Now()
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index e699b0cc94..67736c4e7f 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -51,6 +51,7 @@ The sandbox cluster installation is running under the constant load generated by
* BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): retry on import errors in `vm-native` mode. Before, retries happened only on writes into a network connection between source and destination. But errors returned by server after all the data was transmitted were logged, but not retried.
* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly assume role with [AWS IRSA authorization](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html). Previously role chaining was not supported. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3822) for details.
* BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): fix a link for the statistic inaccuracy explanation in the cardinality explorer tool. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5460).
+* BUGFIX: all: fix potential panic during components shutdown when `-pushmetrics.url` is configured. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5548). Thanks to @zhdd99 for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5549).
## [v1.96.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.96.0)
diff --git a/lib/pushmetrics/pushmetrics.go b/lib/pushmetrics/pushmetrics.go
index 46d2be18e8..0e5ddff3f7 100644
--- a/lib/pushmetrics/pushmetrics.go
+++ b/lib/pushmetrics/pushmetrics.go
@@ -29,8 +29,7 @@ func init() {
}
var (
- // create a custom context for the pushmetrics module to close the metric reporting goroutine when the vmstorage process is shutdown.
- pushMetricsCtx, cancelPushMetric = context.WithCancel(context.Background())
+ pushCtx, cancelPushCtx = context.WithCancel(context.Background())
)
// Init must be called after logger.Init
@@ -42,12 +41,17 @@ func Init() {
Headers: *pushHeader,
DisableCompression: *disableCompression,
}
- if err := metrics.InitPushExtWithOptions(pushMetricsCtx, pu, *pushInterval, appmetrics.WritePrometheusMetrics, opts); err != nil {
+ if err := metrics.InitPushExtWithOptions(pushCtx, pu, *pushInterval, appmetrics.WritePrometheusMetrics, opts); err != nil {
logger.Fatalf("cannot initialize pushmetrics: %s", err)
}
}
}
-func StopPushMetrics() {
- cancelPushMetric()
+// Stop stops the periodic push of metrics.
+// It is important to stop the push of metrics before disposing resources
+// these metrics attached to. See related https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5548
+//
+// Stop must be called after Init.
+func Stop() {
+ cancelPushCtx()
}
diff --git a/lib/uint64set/uint64set_test.go b/lib/uint64set/uint64set_test.go
index f6c7ed2794..7b027290c4 100644
--- a/lib/uint64set/uint64set_test.go
+++ b/lib/uint64set/uint64set_test.go
@@ -690,6 +690,40 @@ func testSetSparseItems(t *testing.T, itemsCount int) {
}
}
+func FuzzAddMulti(f *testing.F) {
+ rand.Seed(time.Now().UnixNano())
+
+ f.Add(uint64(10))
+ f.Fuzz(func(t *testing.T, v uint64) {
+ var s1, s2 Set
+
+ v %= 1e3
+ vs := make([]uint64, v)
+ for i := range vs {
+ vs[i] = rand.Uint64()
+ }
+
+ s1.AddMulti(vs)
+ for _, v := range vs {
+ s2.Add(v)
+ }
+ if s1.Len() != s2.Len() {
+ t.Fatalf("unexpected number of items in the set; got %d; want %d\nset:\n%d", s1.Len(), s2.Len(), s1.AppendTo(nil))
+ }
+ for _, x := range vs {
+ if !s1.Has(x) {
+ t.Fatalf("missing item %d in the set", x)
+ }
+ }
+
+ a1 := s1.AppendTo(nil)
+ a2 := s2.AppendTo(nil)
+ if !reflect.DeepEqual(a1, a2) {
+ t.Fatalf("unexpected items in the set;\ngot\n%d\nwant\n%d", a1, a2)
+ }
+ })
+}
+
func TestAddMulti(t *testing.T) {
f := func(a []uint64) {
t.Helper()