Repository: VictoriaMetrics/VictoriaMetrics
Pull request #1278 (2021-05-09 09:41:23)

vmalert: add support for `round_digits` param in datasource package
Starting from v1.56.0 VM supports `round_digits` which allows to limit
the number of digits after the decimal point in response value. The feature
can be used to reduce entropy of produced by recording rules values
and significantly improve the compression. See more details in link below.

https://github.com/VictoriaMetrics/VictoriaMetrics/issues/525

f41gh7: I have question, initial issue mentions also digits round for recording rules at remoteWrite. Is it possible to add it as well, WDYT?
hagen1778: > Btw, round_digits should be already supported as query param for datasource.Url.

Right. Maybe we don't need it as a flag then, users can just add any GET param to datasource.url

Patch:
diff --git a/app/vmalert/alerting.go b/app/vmalert/alerting.go
index 82a53348b..75c80f2de 100644
--- a/app/vmalert/alerting.go
+++ b/app/vmalert/alerting.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"hash/fnv"
 	"sort"
@@ -405,9 +404,6 @@ func alertForToTimeSeries(name string, a *notifier.Alert, timestamp time.Time) p
 	return newTimeSeries(float64(a.Start.Unix()), labels, timestamp)
 }
 
-// ErrStateRestore indicates that the vmalert state failed to restore during startup.
-var ErrStateRestore = errors.New("failed to restore the state")
-
 // Restore restores the state of active alerts basing on previously written timeseries.
 // Restore restores only Start field. Field State will be always Pending and supposed
 // to be updated on next Exec, as well as Value field.
@@ -432,7 +428,7 @@ func (ar *AlertingRule) Restore(ctx context.Context, q datasource.Querier, lookb
 		alertForStateMetricName, ar.Name, labelsFilter, int(lookback.Seconds()))
 	qMetrics, err := q.Query(ctx, expr)
 	if err != nil {
-		return fmt.Errorf("%s: %w", err, ErrStateRestore)
+		return err
 	}
 
 	for _, m := range qMetrics {
diff --git a/app/vmalert/datasource/init.go b/app/vmalert/datasource/init.go
index 62603f749..093dedabb 100644
--- a/app/vmalert/datasource/init.go
+++ b/app/vmalert/datasource/init.go
@@ -4,6 +4,7 @@ import (
 	"flag"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/utils"
 )
@@ -26,6 +27,8 @@ var (
 		"For example, if datasource.queryStep=15s then param \"step\" with value \"15s\" will be added to every query."+
 		"If queryStep isn't specified, rule's evaluationInterval will be used instead.")
 	maxIdleConnections = flag.Int("datasource.maxIdleConnections", 100, `Defines the number of idle (keep-alive connections) to each configured datasource. Consider setting this value equal to the value: groups_total * group.concurrency. Too low a value may result in a high number of sockets in TIME_WAIT state.`)
+	roundDigits        = flag.Int("datasource.roundDigits", 0, `Adds \"round_digits\" GET param to datasource requests. 
+		In VM \"round_digits\" limits the number of digits after the decimal point in response values.`)
 )
 
 // Init creates a Querier from provided flag values.
@@ -33,11 +36,27 @@ func Init() (QuerierBuilder, error) {
 	if *addr == "" {
 		return nil, fmt.Errorf("datasource.url is empty")
 	}
+
 	tr, err := utils.Transport(*addr, *tlsCertFile, *tlsKeyFile, *tlsCAFile, *tlsServerName, *tlsInsecureSkipVerify)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create transport: %w", err)
 	}
 	tr.MaxIdleConns = *maxIdleConnections
-	c := &http.Client{Transport: tr}
-	return NewVMStorage(*addr, *basicAuthUsername, *basicAuthPassword, *lookBack, *queryStep, *appendTypePrefix, c), nil
+
+	var rd string
+	if *roundDigits > 0 {
+		rd = fmt.Sprintf("%d", *roundDigits)
+	}
+
+	return &VMStorage{
+		c:                &http.Client{Transport: tr},
+		basicAuthUser:    *basicAuthUsername,
+		basicAuthPass:    *basicAuthPassword,
+		datasourceURL:    strings.TrimSuffix(*addr, "/"),
+		appendTypePrefix: *appendTypePrefix,
+		lookBack:         *lookBack,
+		queryStep:        *queryStep,
+		roundDigits:      rd,
+		dataSourceType:   NewPrometheusType(),
+	}, nil
 }
diff --git a/app/vmalert/datasource/vm.go b/app/vmalert/datasource/vm.go
index 72ab55175..8b59b389c 100644
--- a/app/vmalert/datasource/vm.go
+++ b/app/vmalert/datasource/vm.go
@@ -81,6 +81,7 @@ type VMStorage struct {
 	appendTypePrefix bool
 	lookBack         time.Duration
 	queryStep        time.Duration
+	roundDigits      string
 
 	dataSourceType     Type
 	evaluationInterval time.Duration
@@ -218,6 +219,9 @@ func (s *VMStorage) setPrometheusReqParams(r *http.Request, query string, timest
 		// override step with user-specified value
 		q.Set("step", s.queryStep.String())
 	}
+	if s.roundDigits != "" {
+		q.Set("round_digits", s.roundDigits)
+	}
 	r.URL.RawQuery = q.Encode()
 }
 
diff --git a/app/vmalert/manager.go b/app/vmalert/manager.go
index 22de88ae8..4d9b0595e 100644
--- a/app/vmalert/manager.go
+++ b/app/vmalert/manager.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"strings"
 	"sync"
@@ -52,12 +51,7 @@ func (m *manager) AlertAPI(gID, aID uint64) (*APIAlert, error) {
 }
 
 func (m *manager) start(ctx context.Context, path []string, validateTpl, validateExpr bool) error {
-	err := m.update(ctx, path, validateTpl, validateExpr, true)
-	if *remoteReadIgnoreRestoreErrors && errors.Is(err, ErrStateRestore) {
-		logger.Errorf("%s", err)
-		return nil
-	}
-	return err
+	return m.update(ctx, path, validateTpl, validateExpr, true)
 }
 
 func (m *manager) close() {
@@ -74,7 +68,10 @@ func (m *manager) startGroup(ctx context.Context, group *Group, restore bool) er
 	if restore && m.rr != nil {
 		err := group.Restore(ctx, m.rr, *remoteReadLookBack, m.labels)
 		if err != nil {
-			return fmt.Errorf("error while restoring state for group %q: %w", group.Name, err)
+			if !*remoteReadIgnoreRestoreErrors {
+				return fmt.Errorf("failed to restore state for group %q: %w", group.Name, err)
+			}
+			logger.Errorf("error while restoring state for group %q: %s", group.Name, err)
 		}
 	}
 
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 80a8520f0..f98da150a 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -11,6 +11,7 @@ sort: 15
 * BUGFIX: properly remove stale parts outside the configured retention if `-retentionPeriod` is smaller than one month. Previously stale parts could remain active for up to a month after they go outside the retention.
 * BUGFIX: stop the process on panic errors, since such errors may leave the process in inconsistent state. Previously panics could be recovered, which could result in unexpected hard-to-debug further behavior of running process.
 * BUGFIX: vminsert, vmagent: make sure data ingestion connections are closed before completing graceful shutdown. Previously the connection may remain open, which could result in trailing samples loss.
+* BUGFIX: vmalert: fix a bug, which could prevent from proper state restore on startup. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1279).
 
 
 ## tip


Test patch:
diff --git a/app/vmalert/datasource/vm_test.go b/app/vmalert/datasource/vm_test.go
index db030cd5a..9a004bcc5 100644
--- a/app/vmalert/datasource/vm_test.go
+++ b/app/vmalert/datasource/vm_test.go
@@ -243,6 +243,16 @@ func TestPrepareReq(t *testing.T) {
 				checkEqualString(t, exp, r.URL.RawQuery)
 			},
 		},
+		{
+			"round digits",
+			&VMStorage{
+				roundDigits: "10",
+			},
+			func(t *testing.T, r *http.Request) {
+				exp := fmt.Sprintf("query=%s&round_digits=10&time=%d", query, timestamp.Unix())
+				checkEqualString(t, exp, r.URL.RawQuery)
+			},
+		},
 	}
 
 	for _, tc := range testCases {