diff --git a/scripts/comparator/compare.go b/scripts/comparator/compare.go index 1c25a858d9..bf04070b54 100644 --- a/scripts/comparator/compare.go +++ b/scripts/comparator/compare.go @@ -53,7 +53,7 @@ func main() { pQueryFile = flag.String("input", "", "the query file") pPromAddress = flag.String("promAdress", "0.0.0.0:9090", "prom address") - pQueryAddress = flag.String("queryAddress", "0.0.0.0:7201", "query address") + pQueryAddress = flag.String("queryAddress", "0.0.0.0:7201/m3query", "M3 query address") pComparatorAddress = flag.String("comparator", "", "comparator address") pRegressionDir = flag.String("regressionDir", "", "optional directory for regression tests") diff --git a/scripts/comparator/regression_data/non_monotonic_buckets.json b/scripts/comparator/regression_data/non_monotonic_buckets.json new file mode 100644 index 0000000000..61e87e5213 --- /dev/null +++ b/scripts/comparator/regression_data/non_monotonic_buckets.json @@ -0,0 +1,93 @@ +{ + "name": "Prometheus compatible workaround for nonmonotonic histogram buckets", + "query": "histogram_quantile(0.99, nonmonotonic_bucket)", + "startMillis": 1584825526, + "endMillis": 1584828792, + "step": 2, + "data": [ + { + "start": "2020-03-21T21:23:32Z", + "end": "2020-03-21T22:08:20Z", + "tags": [ + ["__name__", "nonmonotonic_bucket"], + ["le", "0.1"] + ], + "datapoints": [ + { + "val": "1", + "ts": "2020-03-21T21:23:34.269Z" + } + ] + }, + { + "start": "2020-03-21T21:23:32Z", + "end": "2020-03-21T22:08:20Z", + "tags": [ + ["__name__", "nonmonotonic_bucket"], + ["le", "1"] + ], + "datapoints": [ + { + "val": "9", + "ts": "2020-03-21T21:23:34.269Z" + } + ] + }, + { + "start": "2020-03-21T21:23:32Z", + "end": "2020-03-21T22:08:20Z", + "tags": [ + ["__name__", "nonmonotonic_bucket"], + ["le", "10"] + ], + "datapoints": [ + { + "val": "8", + "ts": "2020-03-21T21:23:34.269Z" + } + ] + }, + { + "start": "2020-03-21T21:23:32Z", + "end": "2020-03-21T22:08:20Z", + "tags": [ + ["__name__", "nonmonotonic_bucket"], + ["le", "100"] + ], + "datapoints": [ + { + "val": "8", + "ts": "2020-03-21T21:23:34.269Z" + } + ] + }, + { + "start": "2020-03-21T21:23:32Z", + "end": "2020-03-21T22:08:20Z", + "tags": [ + ["__name__", "nonmonotonic_bucket"], + ["le", "1000"] + ], + "datapoints": [ + { + "val": "9", + "ts": "2020-03-21T21:23:34.269Z" + } + ] + }, + { + "start": "2020-03-21T21:23:32Z", + "end": "2020-03-21T22:08:20Z", + "tags": [ + ["__name__", "nonmonotonic_bucket"], + ["le", "+Inf"] + ], + "datapoints": [ + { + "val": "9", + "ts": "2020-03-21T21:23:34.269Z" + } + ] + } + ] + } diff --git a/src/query/functions/linear/histogram_quantile.go b/src/query/functions/linear/histogram_quantile.go index d2f56c90ba..6316c4eb60 100644 --- a/src/query/functions/linear/histogram_quantile.go +++ b/src/query/functions/linear/histogram_quantile.go @@ -168,23 +168,23 @@ func gatherSeriesToBuckets(metas []block.SeriesMeta) validSeriesBuckets { excludeTags := [][]byte{tags.Opts.MetricName(), tags.Opts.BucketName()} tagsWithoutKeys := tags.TagsWithoutKeys(excludeTags) - id := tagsWithoutKeys.ID() + id := string(tagsWithoutKeys.ID()) newBucket := indexedBucket{ upperBound: bound, idx: i, } - if buckets, found := bucketsForID[string(id)]; !found { + if buckets, found := bucketsForID[id]; !found { // add a single indexed bucket for this ID with the current index only. newBuckets := make([]indexedBucket, 0, initIndexBucketLength) newBuckets = append(newBuckets, newBucket) - bucketsForID[string(id)] = indexedBuckets{ + bucketsForID[id] = indexedBuckets{ buckets: newBuckets, tags: tagsWithoutKeys, } } else { buckets.buckets = append(buckets.buckets, newBucket) - bucketsForID[string(id)] = buckets + bucketsForID[id] = buckets } } @@ -316,6 +316,20 @@ func setupBuilder( return builder, nil } +// Enforce monotonicity for binary search to work. +// See https://github.com/prometheus/prometheus/commit/896f951e6846ce252d9d19fd4707a4110ceda5ee +func ensureMonotonic(bucketValues []bucketValue) { + max := math.Inf(-1) + for i := range bucketValues { + switch { + case bucketValues[i].value >= max: + max = bucketValues[i].value + case bucketValues[i].value < max: + bucketValues[i].value = max + } + } +} + func processValidQuantile( queryCtx *models.QueryContext, q float64, @@ -352,6 +366,8 @@ func processValidQuantile( } } + ensureMonotonic(bucketValues) + aggregatedValues = append(aggregatedValues, bucketQuantile(q, bucketValues)) } diff --git a/src/query/functions/linear/histogram_quantile_test.go b/src/query/functions/linear/histogram_quantile_test.go index b328568210..70a648e780 100644 --- a/src/query/functions/linear/histogram_quantile_test.go +++ b/src/query/functions/linear/histogram_quantile_test.go @@ -146,6 +146,72 @@ func TestSanitizeBuckets(t *testing.T) { assert.Equal(t, expected, sanitizeBuckets(bucketed)) } +func TestEnsureMonotonic(t *testing.T) { + tests := []struct { + name string + data []bucketValue + want []bucketValue + }{ + { + "empty", + []bucketValue{}, + []bucketValue{}, + }, + { + "one", + []bucketValue{{upperBound: 1, value: 5}}, + []bucketValue{{upperBound: 1, value: 5}}, + }, + { + "two monotonic", + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 6}}, + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 6}}, + }, + { + "two nonmonotonic", + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 4}}, + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 5}}, + }, + { + "three monotonic", + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 6}, {upperBound: 3, value: 7}}, + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 6}, {upperBound: 3, value: 7}}, + }, + { + "three nonmonotonic", + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 3}, {upperBound: 3, value: 4}}, + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 5}, {upperBound: 3, value: 5}}, + }, + { + "four nonmonotonic", + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 3}, {upperBound: 3, value: 6}, {upperBound: 4, value: 3}}, + []bucketValue{{upperBound: 1, value: 5}, {upperBound: 2, value: 5}, {upperBound: 3, value: 6}, {upperBound: 4, value: 6}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ensureMonotonic(tt.data) + assert.Equal(t, tt.want, tt.data) + }) + } +} + +func TestEnsureMonotonicPreserveNaN(t *testing.T) { + data := []bucketValue{ + {upperBound: 1, value: 5}, + {upperBound: 2, value: 3}, + {upperBound: 3, value: math.NaN()}, + {upperBound: 4, value: 0}, + } + ensureMonotonic(data) + assert.Equal(t, data[0], bucketValue{upperBound: 1, value: 5}) + assert.Equal(t, data[1], bucketValue{upperBound: 2, value: 5}) + assert.Equal(t, data[2].upperBound, float64(3)) + assert.True(t, math.IsNaN(data[2].value)) + assert.Equal(t, data[3], bucketValue{upperBound: 4, value: 5}) +} + func TestBucketQuantile(t *testing.T) { // single bucket returns nan actual := bucketQuantile(0.5, []bucketValue{{upperBound: 1, value: 1}})