From 8d7d8f0125134a4e1e96247dad2a0639ac7d788e Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 16 Dec 2022 17:12:26 -0800 Subject: [PATCH] app/vmselect/prometheus: follow-up after 86dae56bd0296c32962bc84c6c6c07b124bd67f2 Return error if the provided latency_offset query arg cannot be parsed. This should simplify debugging in production. Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3481 --- app/vmselect/prometheus/prometheus.go | 18 +++++----- app/vmselect/prometheus/prometheus_test.go | 40 ++++++++++++++++------ docs/CHANGELOG.md | 5 +-- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/app/vmselect/prometheus/prometheus.go b/app/vmselect/prometheus/prometheus.go index ac7bcaebd2..0454499e51 100644 --- a/app/vmselect/prometheus/prometheus.go +++ b/app/vmselect/prometheus/prometheus.go @@ -831,7 +831,10 @@ func QueryHandler(qt *querytracer.Tracer, startTime time.Time, at *auth.Token, w return nil } - queryOffset := getLatencyOffsetMilliseconds(r) + queryOffset, err := getLatencyOffsetMilliseconds(r) + if err != nil { + return err + } if !searchutils.GetBool(r, "nocache") && ct-start < queryOffset && start-ct < queryOffset { // Adjust start time only if `nocache` arg isn't set. // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/241 @@ -962,7 +965,10 @@ func queryRangeHandler(qt *querytracer.Tracer, startTime time.Time, at *auth.Tok return err } if step < maxStepForPointsAdjustment.Milliseconds() { - queryOffset := getLatencyOffsetMilliseconds(r) + queryOffset, err := getLatencyOffsetMilliseconds(r) + if err != nil { + return err + } if ct-queryOffset < end { result = adjustLastPoints(result, ct-queryOffset, ct+step) } @@ -1102,16 +1108,12 @@ func getRoundDigits(r *http.Request) int { return n } -func getLatencyOffsetMilliseconds(r *http.Request) int64 { +func getLatencyOffsetMilliseconds(r *http.Request) (int64, error) { d := latencyOffset.Milliseconds() if d <= 1000 { d = 1000 } - lo, err := searchutils.GetDuration(r, "latency_offset", d) - if err != nil { - return d - } - return lo + return searchutils.GetDuration(r, "latency_offset", d) } // QueryStatsHandler returns query stats at `/api/v1/status/top_queries` diff --git a/app/vmselect/prometheus/prometheus_test.go b/app/vmselect/prometheus/prometheus_test.go index ae08ce31d1..b740ae6521 100644 --- a/app/vmselect/prometheus/prometheus_test.go +++ b/app/vmselect/prometheus/prometheus_test.go @@ -5,7 +5,6 @@ import ( "net/http" "reflect" "testing" - "time" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmselect/netstorage" ) @@ -198,16 +197,35 @@ func TestAdjustLastPoints(t *testing.T) { }) } -func TestGetLatencyOffsetMilliseconds(t *testing.T) { - f := func(got, exp int64) { - if got != exp { - t.Fatalf("expected offset %d; got %d", exp, got) +func TestGetLatencyOffsetMillisecondsSuccess(t *testing.T) { + f := func(url string, expectedOffset int64) { + t.Helper() + r, err := http.NewRequest("GET", url, nil) + if err != nil { + t.Fatalf("unexpected error in NewRequest(%q): %s", url, err) + } + offset, err := getLatencyOffsetMilliseconds(r) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if offset != expectedOffset { + t.Fatalf("unexpected offset got %d; want %d", offset, expectedOffset) } } - - r, _ := http.NewRequest(http.MethodGet, "http://localhost", nil) - f(getLatencyOffsetMilliseconds(r), latencyOffset.Milliseconds()) - - r, _ = http.NewRequest(http.MethodGet, "http://localhost?latency_offset=10s", nil) - f(getLatencyOffsetMilliseconds(r), 10*time.Second.Milliseconds()) + f("http://localhost", latencyOffset.Milliseconds()) + f("http://localhost?latency_offset=1.234s", 1234) +} + +func TestGetLatencyOffsetMillisecondsFailure(t *testing.T) { + f := func(url string) { + t.Helper() + r, err := http.NewRequest("GET", url, nil) + if err != nil { + t.Fatalf("unexpected error in NewRequest(%q): %s", url, err) + } + if _, err := getLatencyOffsetMilliseconds(r); err == nil { + t.Fatalf("expecting non-nil error") + } + } + f("http://localhost?latency_offset=foobar") } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 63e24f67c1..89fd09be47 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -15,10 +15,11 @@ The following tip changes can be tested by building VictoriaMetrics components f ## tip -* FEATURE: `vmselect`: support overriding of `-search.latencyOffset` value via URL param `latency_offset`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3481). - +* FEATURE: support overriding of `-search.latencyOffset` value via URL param `latency_offset` when performing requests to [/api/v1/query](https://docs.victoriametrics.com/keyConcepts.html#instant-query) and [/api/v1/query_range](https://docs.victoriametrics.com/keyConcepts.html#range-query). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3481). + * BUGFIX: allow specifying values bigger than 2GiB to the following command-line flag values on 32-bit architectures (`386` and `arm`): `-storage.minFreeDiskSpaceBytes` and `-remoteWrite.maxDiskUsagePerURL`. Previously values bigger than 2GiB were incorrectly truncated on these architectures. * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): stop dropping metric name by a mistake on the [/metric-relabel-debug](https://docs.victoriametrics.com/vmagent.html#relabel-debug) page. + * BUGFIX: allow specifying values bigger than 2GiB to the following command-line flag values on 32-bit architectures (`386` and `arm`): `-storage.minFreeDiskSpaceBytes` and `-remoteWrite.maxDiskUsagePerURL`. Previously values bigger than 2GiB were incorrectly truncated on these architectures. ## [v1.85.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.85.1)