Repository: VictoriaMetrics/VictoriaMetrics
Pull request #1027 (2021-01-21 14:19:34)

vmalert: add option datasource.queryStep to allow user to address the…
… inconsistency between grafana dashboards(query_range with step 15s usually) and ALERTS or recording-rules

this should help https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1025


Patch:
diff --git a/app/vmalert/datasource/init.go b/app/vmalert/datasource/init.go
index 6658e3d28..9c9546dea 100644
--- a/app/vmalert/datasource/init.go
+++ b/app/vmalert/datasource/init.go
@@ -24,6 +24,8 @@ var (
 
 	lookBack = flag.Duration("datasource.lookback", 0, "Lookback defines how far to look into past when evaluating queries. "+
 		"For example, if datasource.lookback=5m then param \"time\" with value now()-5m will be added to every query.")
+	queryStep = flag.Duration("datasource.queryStep", 0, "queryStep defines how far a value can fallback to when evaluating queries. "+
+		"For example, if datasource.queryStep=15s then param \"step\" with value \"15s\" will be added to every query.")
 	maxIdleConnections = flag.Int("datasource.maxIdleConnections", 100, "Defines the number of idle (keep-alive connections) to configured datasource."+
 		"Consider to set this value equal to the value: groups_total * group.concurrency. Too low value may result into high number of sockets in TIME_WAIT state.")
 )
@@ -39,5 +41,5 @@ func Init() (Querier, error) {
 	}
 	tr.MaxIdleConns = *maxIdleConnections
 	c := &http.Client{Transport: tr}
-	return NewVMStorage(*addr, *basicAuthUsername, *basicAuthPassword, *lookBack, c), nil
+	return NewVMStorage(*addr, *basicAuthUsername, *basicAuthPassword, *lookBack, *queryStep, c), nil
 }
diff --git a/app/vmalert/datasource/vm.go b/app/vmalert/datasource/vm.go
index 7e0bc4ce4..ae99df121 100644
--- a/app/vmalert/datasource/vm.go
+++ b/app/vmalert/datasource/vm.go
@@ -53,18 +53,20 @@ type VMStorage struct {
 	basicAuthUser string
 	basicAuthPass string
 	lookBack      time.Duration
+	queryStep     time.Duration
 }
 
 const queryPath = "/api/v1/query?query="
 
 // NewVMStorage is a constructor for VMStorage
-func NewVMStorage(baseURL, basicAuthUser, basicAuthPass string, lookBack time.Duration, c *http.Client) *VMStorage {
+func NewVMStorage(baseURL, basicAuthUser, basicAuthPass string, lookBack time.Duration, queryStep time.Duration, c *http.Client) *VMStorage {
 	return &VMStorage{
 		c:             c,
 		basicAuthUser: basicAuthUser,
 		basicAuthPass: basicAuthPass,
 		queryURL:      strings.TrimSuffix(baseURL, "/") + queryPath,
 		lookBack:      lookBack,
+		queryStep:     queryStep,
 	}
 }
 
@@ -78,6 +80,9 @@ func (s *VMStorage) Query(ctx context.Context, query string) ([]Metric, error) {
 		lookBack := time.Now().Add(-s.lookBack)
 		q += fmt.Sprintf("&time=%d", lookBack.Unix())
 	}
+	if s.queryStep > 0 {
+		q += fmt.Sprintf("&step=%s", s.queryStep.String())
+	}
 	req, err := http.NewRequest("POST", q, nil)
 	if err != nil {
 		return nil, err
diff --git a/app/vmalert/remoteread/init.go b/app/vmalert/remoteread/init.go
index dfbcba8f6..7a0aa32ac 100644
--- a/app/vmalert/remoteread/init.go
+++ b/app/vmalert/remoteread/init.go
@@ -35,5 +35,5 @@ func Init() (datasource.Querier, error) {
 		return nil, fmt.Errorf("failed to create transport: %w", err)
 	}
 	c := &http.Client{Transport: tr}
-	return datasource.NewVMStorage(*addr, *basicAuthUsername, *basicAuthPassword, 0, c), nil
+	return datasource.NewVMStorage(*addr, *basicAuthUsername, *basicAuthPassword, 0, 0, c), nil
 }
diff --git a/app/vmselect/promql/eval.go b/app/vmselect/promql/eval.go
index c92381579..e5735c72e 100644
--- a/app/vmselect/promql/eval.go
+++ b/app/vmselect/promql/eval.go
@@ -19,7 +19,9 @@ import (
 
 var (
 	disableCache           = flag.Bool("search.disableCache", false, "Whether to disable response caching. This may be useful during data backfilling")
-	maxPointsPerTimeseries = flag.Int("search.maxPointsPerTimeseries", 30e3, "The maximum points per a single timeseries returned from the search")
+	maxPointsPerTimeseries = flag.Int("search.maxPointsPerTimeseries", 30e3, "The maximum points per a single timeseries returned from /api/v1/query_range. "+
+		"This option doesn't limit the number of scanned raw samples in the database. The main purpose of this option is to limit the number of per-series points "+
+		"returned to graphing UI such as Grafana. There is no sense in setting this limit to values significantly exceeding horizontal resoultion of the graph")
 )
 
 // The minimum number of points per timeseries for enabling time rounding.
diff --git a/deployment/docker/Makefile b/deployment/docker/Makefile
index 0e13ca21c..dd28c0590 100644
--- a/deployment/docker/Makefile
+++ b/deployment/docker/Makefile
@@ -4,7 +4,7 @@ DOCKER_NAMESPACE := victoriametrics
 
 ROOT_IMAGE ?= alpine:3.13.0
 CERTS_IMAGE := alpine:3.13.0
-GO_BUILDER_IMAGE := golang:1.15.6
+GO_BUILDER_IMAGE := golang:1.15.7
 BUILDER_IMAGE := local/builder:2.0.0-$(shell echo $(GO_BUILDER_IMAGE) | tr : _)
 BASE_IMAGE := local/base:1.1.1-$(shell echo $(ROOT_IMAGE) | tr : _)-$(shell echo $(CERTS_IMAGE) | tr : _)
 
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 37cf62e9d..d8e01d4b2 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -4,6 +4,9 @@
 
 * FEATURE: added `search.maxStepForPointsAdjustment` command-line flag, which can be used for disabling adjustment for points returned `/api/v1/query_range` handler if such points have timestamps closer than `-search.latencyOffset` to the current time. Such points may contain incomplete data, so they are substituted by the previous values for `step` query args smaller than one minute by default.
 
+* BUGFIX: vmagent: reduce the HTTP reconnection rate to scrape targets. Previously vmagent could errorneusly close HTTP keep-alive connections more often than needed.
+* BUGFIX: vmagent: retry scrape and service discovery requests when the remote server closes HTTP keep-alive connection. Previously `disable_keepalive: true` option could be used under `scrape_configs` section when working with such servers.
+
 
 # [v1.52.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.52.0)
 
diff --git a/lib/promscrape/client.go b/lib/promscrape/client.go
index bec3ef23e..03cbcf193 100644
--- a/lib/promscrape/client.go
+++ b/lib/promscrape/client.go
@@ -252,7 +252,7 @@ func doRequestWithPossibleRetry(hc *fasthttp.HostClient, req *fasthttp.Request,
 		if err == nil {
 			return nil
 		}
-		if err != fasthttp.ErrConnectionClosed {
+		if err != fasthttp.ErrConnectionClosed && !strings.Contains(err.Error(), "broken pipe") {
 			return err
 		}
 		// Retry request if the server closes the keep-alive connection unless deadline exceeds.
diff --git a/lib/promscrape/discoveryutils/client.go b/lib/promscrape/discoveryutils/client.go
index b971be8ec..0acdace38 100644
--- a/lib/promscrape/discoveryutils/client.go
+++ b/lib/promscrape/discoveryutils/client.go
@@ -199,7 +199,7 @@ func doRequestWithPossibleRetry(hc *fasthttp.HostClient, req *fasthttp.Request,
 		if err == nil {
 			return nil
 		}
-		if err != fasthttp.ErrConnectionClosed {
+		if err != fasthttp.ErrConnectionClosed && !strings.Contains(err.Error(), "broken pipe") {
 			return err
 		}
 		// Retry request if the server closes the keep-alive connection unless deadline exceeds.


Test patch:
diff --git a/app/vmalert/datasource/vm_test.go b/app/vmalert/datasource/vm_test.go
index 4c59129ae..af99c8421 100644
--- a/app/vmalert/datasource/vm_test.go
+++ b/app/vmalert/datasource/vm_test.go
@@ -61,7 +61,7 @@ func TestVMSelectQuery(t *testing.T) {
 
 	srv := httptest.NewServer(mux)
 	defer srv.Close()
-	am := NewVMStorage(srv.URL, basicAuthName, basicAuthPass, time.Minute, srv.Client())
+	am := NewVMStorage(srv.URL, basicAuthName, basicAuthPass, time.Minute, 0, srv.Client())
 	if _, err := am.Query(ctx, query); err == nil {
 		t.Fatalf("expected connection error got nil")
 	}