From a5285dd8d388ce278d1d6ac7ada477347482218b Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Wed, 11 Sep 2024 19:55:51 -0700 Subject: [PATCH] MQE: Native histogram lookback should re-use existing histograms (#9272) (#9276) * MQE: Native histogram lookback should re-use existing histograms As per the code comments, the consumer is expected to check for repeated histograms. * Update CHANGELOG * Add extra check (cherry picked from commit db75986fa643f0c6f301e0296a1c12b7a0204805) Co-authored-by: Joshua Hesketh --- CHANGELOG.md | 2 +- .../operators/instant_vector_selector.go | 7 ++----- .../operators/instant_vector_selector_test.go | 13 +++++++++++++ pkg/streamingpromql/testdata/ours/aggregators.test | 7 +++++++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e48c0b88918..8055d0ce98a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,7 @@ * [CHANGE] Query-scheduler: Remove the experimental `-query-scheduler.use-multi-algorithm-query-queue` flag. The new multi-algorithm tree queue is always used for the scheduler. #9210 * [FEATURE] Alertmanager: Added `-alertmanager.log-parsing-label-matchers` to control logging when parsing label matchers. This flag is intended to be used with `-alertmanager.utf8-strict-mode-enabled` to validate UTF-8 strict mode is working as intended. The default value is `false`. #9173 * [FEATURE] Alertmanager: Added `-alertmanager.utf8-migration-logging-enabled` to enable logging of tenant configurations that are incompatible with UTF-8 strict mode. The default value is `false`. #9174 -* [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #8422 #8430 #8454 #8455 #8360 #8490 #8508 #8577 #8660 #8671 #8677 #8747 #8850 #8872 #8838 #8911 #8909 #8923 #8924 #8925 #8932 #8933 #8934 #8962 #8986 #8993 #8995 #9008 #9017 #9018 #9019 #9120 #9121 #9136 #9139 #9140 #9145 #9191 #9192 #9194 #9196 #9201 #9212 #9225 #9260 +* [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #8422 #8430 #8454 #8455 #8360 #8490 #8508 #8577 #8660 #8671 #8677 #8747 #8850 #8872 #8838 #8911 #8909 #8923 #8924 #8925 #8932 #8933 #8934 #8962 #8986 #8993 #8995 #9008 #9017 #9018 #9019 #9120 #9121 #9136 #9139 #9140 #9145 #9191 #9192 #9194 #9196 #9201 #9212 #9225 #9260 #9272 * [FEATURE] Experimental Kafka-based ingest storage. #6888 #6894 #6929 #6940 #6951 #6974 #6982 #7029 #7030 #7091 #7142 #7147 #7148 #7153 #7160 #7193 #7349 #7376 #7388 #7391 #7393 #7394 #7402 #7404 #7423 #7424 #7437 #7486 #7503 #7508 #7540 #7621 #7682 #7685 #7694 #7695 #7696 #7697 #7701 #7733 #7734 #7741 #7752 #7838 #7851 #7871 #7877 #7880 #7882 #7887 #7891 #7925 #7955 #7967 #8031 #8063 #8077 #8088 #8135 #8176 #8184 #8194 #8216 #8217 #8222 #8233 #8503 #8542 #8579 #8657 #8686 #8688 #8703 #8706 #8708 #8738 #8750 #8778 #8808 #8809 #8841 #8842 #8845 #8853 #8886 #8988 * What it is: * When the new ingest storage architecture is enabled, distributors write incoming write requests to a Kafka-compatible backend, and the ingesters asynchronously replay ingested data from Kafka. In this architecture, the write and read path are de-coupled through a Kafka-compatible backend. The write path and Kafka load is a function of the incoming write traffic, the read path load is a function of received queries. Whatever the load on the read path, it doesn't affect the write path. diff --git a/pkg/streamingpromql/operators/instant_vector_selector.go b/pkg/streamingpromql/operators/instant_vector_selector.go index 443c8b0a25c..8282c58a50d 100644 --- a/pkg/streamingpromql/operators/instant_vector_selector.go +++ b/pkg/streamingpromql/operators/instant_vector_selector.go @@ -100,8 +100,6 @@ func (v *InstantVectorSelector) NextSeries(ctx context.Context) (types.InstantVe t, h = atT, lastHistogram } else { t, h = v.memoizedIterator.AtFloatHistogram() - lastHistogramT = t - lastHistogram = h } default: @@ -123,9 +121,6 @@ func (v *InstantVectorSelector) NextSeries(ctx context.Context) (types.InstantVe // it if they are going to mutate it, but consuming operators don't check the underlying bucket slices, so without // this, we can end up with incorrect query results. h = lastHistogram - } else { - lastHistogramT = t - lastHistogram = h } } } @@ -147,6 +142,8 @@ func (v *InstantVectorSelector) NextSeries(ctx context.Context) (types.InstantVe } } data.Histograms = append(data.Histograms, promql.HPoint{T: stepT, H: h}) + lastHistogramT = t + lastHistogram = h } else { if len(data.Floats) == 0 { // Only create the slice once we know the series is a histogram or not diff --git a/pkg/streamingpromql/operators/instant_vector_selector_test.go b/pkg/streamingpromql/operators/instant_vector_selector_test.go index 7263ec8e2fd..9248ad048e0 100644 --- a/pkg/streamingpromql/operators/instant_vector_selector_test.go +++ b/pkg/streamingpromql/operators/instant_vector_selector_test.go @@ -175,6 +175,19 @@ func TestInstantVectorSelector_NativeHistogramPointerHandling(t *testing.T) { requireNotSame(t, points[0].H, points[1].H) }, }, + "lookback points in middle of series reuse existing histogram": { + data: ` + load 1m + my_metric _ {{schema:5 sum:10 count:7 buckets:[1 2 3 1]}} _ {{schema:5 sum:12 count:8 buckets:[1 2 3 2]}} _ + `, + stepCount: 5, + check: func(t *testing.T, points []promql.HPoint, _ []promql.FPoint) { + require.Len(t, points, 4) + requireNotSame(t, points[0].H, points[2].H) + require.Same(t, points[0].H, points[1].H) + require.Same(t, points[2].H, points[3].H) + }, + }, // FIXME: this test currently fails due to https://github.com/prometheus/prometheus/issues/14172 // //"point has same value as a previous point, but there is a float value in between": { diff --git a/pkg/streamingpromql/testdata/ours/aggregators.test b/pkg/streamingpromql/testdata/ours/aggregators.test index 62a5fc172fb..cb149d20c16 100644 --- a/pkg/streamingpromql/testdata/ours/aggregators.test +++ b/pkg/streamingpromql/testdata/ours/aggregators.test @@ -307,3 +307,10 @@ eval range from 0m to 10m step 5m sum (native_histogram) {} {{schema:3 count:11 sum:14 buckets:[5 6]}} {{schema:3 count:11 sum:12 buckets:[5 6]}} {{schema:5 count:17 sum:18 buckets:[1 5 1]}} clear + +load 1m + series{label="a"} {{schema:5 sum:15 count:10 buckets:[3 2 5]}} {{schema:5 sum:20 count:15 buckets:[4 5 6]}} {{schema:5 sum:25 count:20 buckets:[5 7 8]}} {{schema:5 sum:30 count:25 buckets:[6 9 10]}} {{schema:5 sum:35 count:30 buckets:[7 10 13]}} + series{label="b"} {{schema:4 sum:12 count:8 buckets:[2 3 3]}} {{schema:4 sum:14 count:9 buckets:[3 3 3]}} _ {{schema:4 sum:16 count:10 buckets:[3 4 3]}} {{schema:4 sum:18 count:11 buckets:[4 4 3]}} + +eval range from 0m to 4m step 1m avg(series) + {} {{schema:4 count:9 sum:13.5 buckets:[2.5 5 1.5]}} {{schema:4 count:12 sum:17 buckets:[3.5 7 1.5]}} {{schema:4 count:14.5 sum:19.5 buckets:[4 9 1.5]}} {{schema:4 count:17.5 sum:23 buckets:[4.5 11.5 1.5]}} {{schema:4 count:20.5 sum:26.5 buckets:[5.5 13.5 1.5]}}