Repository: VictoriaMetrics/VictoriaMetrics
Pull request #5897 (2024-02-29 13:03:29)

lib/logstorage: avoid panic when parsing regex with stream filter
We meet a panic when typing in an invalid regex with the stream filter. I think it can be avoided at the `ParseQuery`.

reproduce:

logsql: `_stream: {path=~"*debug.log"}`

```
panic: BUG: cannot parse regexp "*debug.log": error parsing regexp: missing argument to repetition operator: `*`

goroutine 5112 [running]:
github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver.handlerWrapper.func1()
        github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver/httpserver.go:289 +0x5b
panic({0x845320?, 0xc001002740?})
        runtime/panic.go:770 +0x132
github.com/VictoriaMetrics/VictoriaMetrics/lib/logger.logMessage({0x8c006c, 0x5}, {0xc004a89650, 0x69}, 0x4)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logger/logger.go:301 +0xa74
github.com/VictoriaMetrics/VictoriaMetrics/lib/logger.logLevelSkipframes(0x1, {0x8c006c, 0x5}, {0x8ce82d?, 0x42a220?}, {0xc050dd4bc8?, 0x3?, 0xc050dd4b88?})
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logger/logger.go:141 +0x1a5
github.com/VictoriaMetrics/VictoriaMetrics/lib/logger.logLevel(...)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logger/logger.go:133
github.com/VictoriaMetrics/VictoriaMetrics/lib/logger.Panicf(...)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logger/logger.go:129
github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage.(*streamTagFilter).initRegexp(0xc0537fc4b0)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage/stream_filter.go:83 +0xce
sync.(*Once).doSlow(0xc050dd4cb0?, 0x423965?)
        sync/once.go:74 +0xc2
sync.(*Once).Do(...)
        sync/once.go:65
github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage.(*streamTagFilter).getRegexp(...)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage/stream_filter.go:76
github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage.(*indexSearch).getStreamIDsForTagFilter(0xc03101a5a0, {0x854780?, 0x0?}, 0xc0537fc4b0)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage/indexdb.go:270 +0x2d5
github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage.(*indexSearch).updateStreamIDs(0xc03101a5a0, 0xc002767010, {0x1?, 0x0?}, 0x1?)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage/indexdb.go:223 +0xa5
github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage.(*indexdb).searchStreamIDs(0xc00013a230, {0xc004b29208, 0x1, 0x1}, 0xc000b1adc8)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage/indexdb.go:196 +0x368
github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage.(*partition).search(0xc0015e1180, {0x0, 0x0, 0x0}, 0xc00122c460, 0xc0004a8d80?, {0xa7d568, 0xc000b1b500}, 0xc0027672d8, 0xc054cc42a0, ...)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage/storage_search.go:205 +0x90
github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage.(*Storage).search(0xc0000229a0, 0x40, 0xc0027672d8, 0xc002a01b60, 0xc0004a8d80)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage/storage_search.go:179 +0x4d3
github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage.(*Storage).RunQuery(0xc0000229a0, {0xc004b29208, 0x1, 0x1}, 0xc001002650, 0xc002a01b60, 0xc00122c520)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/logstorage/storage_search.go:56 +0x16d
github.com/VictoriaMetrics/VictoriaMetrics/app/vlstorage.RunQuery(...)
        github.com/VictoriaMetrics/VictoriaMetrics/app/vlstorage/main.go:104
github.com/VictoriaMetrics/VictoriaMetrics/app/vlselect/logsql.ProcessQueryRequest({0xa7d238, 0xc000b1ad50}, 0xc0186850e0, 0xc002a01b60, 0xc001002550)
        github.com/VictoriaMetrics/VictoriaMetrics/app/vlselect/logsql/logsql.go:51 +0x4f0
github.com/VictoriaMetrics/VictoriaMetrics/app/vlselect.RequestHandler({0xa7d238, 0xc000b1ad50}, 0xc0186850e0)
        github.com/VictoriaMetrics/VictoriaMetrics/app/vlselect/main.go:152 +0x952
main.requestHandler({0xa7d238, 0xc000b1ad50}, 0xc0186850e0)
        github.com/VictoriaMetrics/VictoriaMetrics/app/victoria-logs/main.go:92 +0x195
github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver.handlerWrapper(0xc00122a4c0, {0xa7dbc8, 0xc036162000}, 0xc0186850e0, 0x9fcf18)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver/httpserver.go:414 +0x1003
github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver.gzipHandler.func1({0xa7dbc8?, 0xc036162000?}, 0xc036162000?)
        github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver/httpserver.go:248 +0x31
net/http.HandlerFunc.ServeHTTP(0xd2d430?, {0xa7dbc8?, 0xc036162000?}, 0x4?)
        net/http/server.go:2166 +0x29
github.com/klauspost/compress/gzhttp.NewWrapper.func1.1({0xa7d118, 0xc02935e7e0}, 0xc0186850e0)
        github.com/klauspost/compress@v1.17.6/gzhttp/compress.go:495 +0x54f
net/http.HandlerFunc.ServeHTTP(0x40e9a5?, {0xa7d118?, 0xc02935e7e0?}, 0xc02935e701?)
        net/http/server.go:2166 +0x29
net/http.serverHandler.ServeHTTP({0xa7c200?}, {0xa7d118?, 0xc02935e7e0?}, 0x6?)
        net/http/server.go:3137 +0x8e
net/http.(*conn).serve(0xc048711560, {0xa7e868, 0xc000fa1710})
        net/http/server.go:2039 +0x5e8
created by net/http.(*Server).Serve in goroutine 4209
        net/http/server.go:3285 +0x4b4
```



codecov[bot]: ## [Codecov](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5897?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) Report
Attention: Patch coverage is `60.00000%` with `2 lines` in your changes are missing coverage. Please review.
> Project coverage is 56.74%. Comparing base [(`8aaa828`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/8aaa828ba3f49a43cb8de89a0bcae160f7988c14?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) to head [(`ccb3720`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5897?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).
> Report is 242 commits behind head on master.

| [Files](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5897?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) | Patch % | Lines |
|---|---|---|
| [lib/logstorage/parser.go](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5897?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-bGliL2xvZ3N0b3JhZ2UvcGFyc2VyLmdv) | 60.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5897?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #5897      +/-   ##
==========================================
- Coverage   60.37%   56.74%   -3.64%     
==========================================
  Files         411      521     +110     
  Lines       76609    71566    -5043     
==========================================
- Hits        46253    40608    -5645     
- Misses      27794    28094     +300     
- Partials     2562     2864     +302     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5897?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).


Patch:
diff --git a/app/vmalert/datasource/vm.go b/app/vmalert/datasource/vm.go
index b1c8e7983..ba8d8cd3c 100644
--- a/app/vmalert/datasource/vm.go
+++ b/app/vmalert/datasource/vm.go
@@ -137,11 +137,11 @@ func NewVMStorage(baseURL string, authCfg *promauth.Config, lookBack time.Durati
 
 // Query executes the given query and returns parsed response
 func (s *VMStorage) Query(ctx context.Context, query string, ts time.Time) (Result, *http.Request, error) {
-	req, err := s.newQueryRequest(query, ts)
+	req, err := s.newQueryRequest(ctx, query, ts)
 	if err != nil {
 		return Result{}, nil, err
 	}
-	resp, err := s.do(ctx, req)
+	resp, err := s.do(req)
 	if err != nil {
 		if !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) {
 			// Return unexpected error to the caller.
@@ -149,11 +149,11 @@ func (s *VMStorage) Query(ctx context.Context, query string, ts time.Time) (Resu
 		}
 		// Something in the middle between client and datasource might be closing
 		// the connection. So we do a one more attempt in hope request will succeed.
-		req, err = s.newQueryRequest(query, ts)
+		req, err = s.newQueryRequest(ctx, query, ts)
 		if err != nil {
 			return Result{}, nil, fmt.Errorf("second attempt: %w", err)
 		}
-		resp, err = s.do(ctx, req)
+		resp, err = s.do(req)
 		if err != nil {
 			return Result{}, nil, fmt.Errorf("second attempt: %w", err)
 		}
@@ -182,11 +182,11 @@ func (s *VMStorage) QueryRange(ctx context.Context, query string, start, end tim
 	if end.IsZero() {
 		return res, fmt.Errorf("end param is missing")
 	}
-	req, err := s.newQueryRangeRequest(query, start, end)
+	req, err := s.newQueryRangeRequest(ctx, query, start, end)
 	if err != nil {
 		return res, err
 	}
-	resp, err := s.do(ctx, req)
+	resp, err := s.do(req)
 	if err != nil {
 		if !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) {
 			// Return unexpected error to the caller.
@@ -194,11 +194,11 @@ func (s *VMStorage) QueryRange(ctx context.Context, query string, start, end tim
 		}
 		// Something in the middle between client and datasource might be closing
 		// the connection. So we do a one more attempt in hope request will succeed.
-		req, err = s.newQueryRangeRequest(query, start, end)
+		req, err = s.newQueryRangeRequest(ctx, query, start, end)
 		if err != nil {
 			return res, fmt.Errorf("second attempt: %w", err)
 		}
-		resp, err = s.do(ctx, req)
+		resp, err = s.do(req)
 		if err != nil {
 			return res, fmt.Errorf("second attempt: %w", err)
 		}
@@ -210,7 +210,7 @@ func (s *VMStorage) QueryRange(ctx context.Context, query string, start, end tim
 	return res, err
 }
 
-func (s *VMStorage) do(ctx context.Context, req *http.Request) (*http.Response, error) {
+func (s *VMStorage) do(req *http.Request) (*http.Response, error) {
 	ru := req.URL.Redacted()
 	if *showDatasourceURL {
 		ru = req.URL.String()
@@ -218,7 +218,7 @@ func (s *VMStorage) do(ctx context.Context, req *http.Request) (*http.Response,
 	if s.debug {
 		logger.Infof("DEBUG datasource request: executing %s request with params %q", req.Method, ru)
 	}
-	resp, err := s.c.Do(req.WithContext(ctx))
+	resp, err := s.c.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("error getting response from %s: %w", ru, err)
 	}
@@ -230,8 +230,8 @@ func (s *VMStorage) do(ctx context.Context, req *http.Request) (*http.Response,
 	return resp, nil
 }
 
-func (s *VMStorage) newQueryRangeRequest(query string, start, end time.Time) (*http.Request, error) {
-	req, err := s.newRequest()
+func (s *VMStorage) newQueryRangeRequest(ctx context.Context, query string, start, end time.Time) (*http.Request, error) {
+	req, err := s.newRequest(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("cannot create query_range request to datasource %q: %w", s.datasourceURL, err)
 	}
@@ -239,8 +239,8 @@ func (s *VMStorage) newQueryRangeRequest(query string, start, end time.Time) (*h
 	return req, nil
 }
 
-func (s *VMStorage) newQueryRequest(query string, ts time.Time) (*http.Request, error) {
-	req, err := s.newRequest()
+func (s *VMStorage) newQueryRequest(ctx context.Context, query string, ts time.Time) (*http.Request, error) {
+	req, err := s.newRequest(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("cannot create query request to datasource %q: %w", s.datasourceURL, err)
 	}
@@ -255,8 +255,8 @@ func (s *VMStorage) newQueryRequest(query string, ts time.Time) (*http.Request,
 	return req, nil
 }
 
-func (s *VMStorage) newRequest() (*http.Request, error) {
-	req, err := http.NewRequest(http.MethodPost, s.datasourceURL, nil)
+func (s *VMStorage) newRequest(ctx context.Context) (*http.Request, error) {
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, s.datasourceURL, nil)
 	if err != nil {
 		logger.Panicf("BUG: unexpected error from http.NewRequest(%q): %s", s.datasourceURL, err)
 	}
diff --git a/app/vmalert/remotewrite/client.go b/app/vmalert/remotewrite/client.go
index 2c9fc1ac1..de9ac6abc 100644
--- a/app/vmalert/remotewrite/client.go
+++ b/app/vmalert/remotewrite/client.go
@@ -279,7 +279,7 @@ L:
 
 func (c *Client) send(ctx context.Context, data []byte) error {
 	r := bytes.NewReader(data)
-	req, err := http.NewRequest(http.MethodPost, c.addr, r)
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.addr, r)
 	if err != nil {
 		return fmt.Errorf("failed to create new HTTP request: %w", err)
 	}
@@ -302,7 +302,7 @@ func (c *Client) send(ctx context.Context, data []byte) error {
 	if !*disablePathAppend {
 		req.URL.Path = path.Join(req.URL.Path, "/api/v1/write")
 	}
-	resp, err := c.c.Do(req.WithContext(ctx))
+	resp, err := c.c.Do(req)
 	if err != nil {
 		return fmt.Errorf("error while sending request to %s: %w; Data len %d(%d)",
 			req.URL.Redacted(), err, len(data), r.Size())
diff --git a/app/vmctl/remoteread/remoteread.go b/app/vmctl/remoteread/remoteread.go
index 349fbb165..e0fcc19f9 100644
--- a/app/vmctl/remoteread/remoteread.go
+++ b/app/vmctl/remoteread/remoteread.go
@@ -182,7 +182,7 @@ func (c *Client) fetch(ctx context.Context, data []byte, streamCb StreamCallback
 	if c.disablePathAppend {
 		u = c.addr
 	}
-	req, err := http.NewRequest(http.MethodPost, u, r)
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, u, r)
 	if err != nil {
 		return fmt.Errorf("failed to create new HTTP request: %w", err)
 	}
@@ -195,7 +195,7 @@ func (c *Client) fetch(ctx context.Context, data []byte, streamCb StreamCallback
 	}
 	req.Header.Set("X-Prometheus-Remote-Read-Version", "0.1.0")
 
-	resp, err := c.do(req.WithContext(ctx))
+	resp, err := c.do(req)
 	if err != nil {
 		return fmt.Errorf("error while sending request to %s: %w; Data len %d(%d)",
 			req.URL.Redacted(), err, len(data), r.Size())
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 8075ae52c..d8daa29cf 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -43,6 +43,7 @@ See also [LTS releases](https://docs.victoriametrics.com/LTS-releases.html).
 * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): preserve [`WITH` templates](https://play.victoriametrics.com/select/accounting/1/6a716b0f-38bc-4856-90ce-448fd713e3fe/expand-with-exprs) when clicking the `prettify query` button at the right side of query input field. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5383).
 * FEATURE: [vmalert](https://docs.victoriametrics.com/#vmalert): support filtering by group, rule or labels in [vmalert's UI](https://docs.victoriametrics.com/vmalert/#web) for `/groups` and `/alerts` pages. See [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5791) by @victoramsantos.
 * FEATURE: [docker-compose](https://github.com/VictoriaMetrics/VictoriaMetrics/tree/master/deployment/docker#docker-compose-environment-for-victoriametrics): create a separate [docker-compose environment](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/docker-compose-victorialogs.yml) for VictoriaLogs installation, including fluentbit and [VictoriaLogs Grafana datasource](https://github.com/VictoriaMetrics/victorialogs-datasource).
+* FEATURE: [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager/): wait for up 30 seconds before making a [snapshot](https://docs.victoriametrics.com/#how-to-work-with-snapshots) for backup if `vmstorage` is temporarily unavailalbe. This should prevent from `vmbackupmanager` termination in this case. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5859).
 
 * BUGFIX: downgrade Go builder from `1.22.0` to `1.21.7`, since `1.22.0` contains [the bug](https://github.com/golang/go/issues/65705), which can lead to deadlocked HTTP connections to remote storage systems, scrape targets and service discovery endpoints at [vmagent](https://docs.victoriametrics.com/vmagent/). This may result in incorrect service discovery, target scraping and failed sending samples to remote storage.
 * BUGFIX: all VictoriaMetrics components: return back periodic closing of incoming connections to `-httpListenAddr` every 2 minutes, which was disabled in [v1.98.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.98.0) when addressing [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1304#issuecomment-1636997037). See [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1304#issuecomment-1961891450) for details on why the periodic closing of incoming connections has been returned back.
diff --git a/lib/logstorage/parser.go b/lib/logstorage/parser.go
index d0f66c898..104491a01 100644
--- a/lib/logstorage/parser.go
+++ b/lib/logstorage/parser.go
@@ -12,6 +12,7 @@ import (
 	"unicode/utf8"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/regexutil"
 )
 
 type lexer struct {
@@ -1070,6 +1071,13 @@ func parseStreamTagFilter(lex *lexer) (*streamTagFilter, error) {
 		op:      op,
 		value:   value,
 	}
+	if op == "=~" || op == "!~" {
+		re, err := regexutil.NewPromRegex(value)
+		if err != nil {
+			return nil, fmt.Errorf("invalid regexp %q for stream filter: %w", value, err)
+		}
+		stf.regexp = re
+	}
 	return stf, nil
 }
 
diff --git a/lib/logstorage/stream_filter.go b/lib/logstorage/stream_filter.go
index 838e95ed0..0cce17229 100644
--- a/lib/logstorage/stream_filter.go
+++ b/lib/logstorage/stream_filter.go
@@ -3,11 +3,9 @@ package logstorage
 import (
 	"strconv"
 	"strings"
-	"sync"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/encoding"
-	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/regexutil"
 )
 
@@ -68,23 +66,13 @@ type streamTagFilter struct {
 	// value is the value
 	value string
 
-	regexpOnce sync.Once
-	regexp     *regexutil.PromRegex
+	regexp *regexutil.PromRegex
 }
 
 func (tf *streamTagFilter) getRegexp() *regexutil.PromRegex {
-	tf.regexpOnce.Do(tf.initRegexp)
 	return tf.regexp
 }
 
-func (tf *streamTagFilter) initRegexp() {
-	re, err := regexutil.NewPromRegex(tf.value)
-	if err != nil {
-		logger.Panicf("BUG: cannot parse regexp %q: %s", tf.value, err)
-	}
-	tf.regexp = re
-}
-
 func (tf *streamTagFilter) String() string {
 	return quoteTokenIfNeeded(tf.tagName) + tf.op + strconv.Quote(tf.value)
 }


Test patch:
diff --git a/app/vmalert/datasource/vm_test.go b/app/vmalert/datasource/vm_test.go
index 769797438..78b38ecc1 100644
--- a/app/vmalert/datasource/vm_test.go
+++ b/app/vmalert/datasource/vm_test.go
@@ -637,7 +637,7 @@ func TestRequestParams(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			req, err := tc.vm.newRequest()
+			req, err := tc.vm.newRequest(ctx)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -735,7 +735,7 @@ func TestHeaders(t *testing.T) {
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
 			vm := tt.vmFn()
-			req, err := vm.newQueryRequest("foo", time.Now())
+			req, err := vm.newQueryRequest(ctx, "foo", time.Now())
 			if err != nil {
 				t.Fatal(err)
 			}