From 750f862d9a501a00629992b9504e49fc29971bca Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Sun, 12 May 2013 02:03:16 +0200 Subject: [PATCH] Use GetBoundaryValues() for non-counter deltas. --- rules/ast/persistence_adapter.go | 3 +- rules/helpers_test.go | 31 +- rules/rules_test.go | 12 + storage/metric/interface.go | 12 +- storage/metric/leveldb.go | 6 +- storage/metric/memory.go | 27 +- storage/metric/rule_integration_test.go | 543 +++++++++--------------- 7 files changed, 270 insertions(+), 364 deletions(-) diff --git a/rules/ast/persistence_adapter.go b/rules/ast/persistence_adapter.go index 391bc3096..14c594821 100644 --- a/rules/ast/persistence_adapter.go +++ b/rules/ast/persistence_adapter.go @@ -125,8 +125,7 @@ func (v *viewAdapter) GetValueAtTime(fingerprints model.Fingerprints, timestamp func (v *viewAdapter) GetBoundaryValues(fingerprints model.Fingerprints, interval *model.Interval) (sampleSets []model.SampleSet, err error) { for _, fingerprint := range fingerprints { - // TODO: change to GetBoundaryValues() once it has the right return type. - samplePairs := v.view.GetRangeValues(fingerprint, *interval) + samplePairs := v.view.GetBoundaryValues(fingerprint, *interval) if samplePairs == nil { continue } diff --git a/rules/helpers_test.go b/rules/helpers_test.go index c1c43719b..ed8c6676d 100644 --- a/rules/helpers_test.go +++ b/rules/helpers_test.go @@ -23,10 +23,8 @@ import ( var testSampleInterval = time.Duration(5) * time.Minute var testStartTime = time.Time{} -func getTestValueStream(startVal model.SampleValue, - endVal model.SampleValue, - stepVal model.SampleValue) (resultValues model.Values) { - currentTime := testStartTime +func getTestValueStream(startVal model.SampleValue, endVal model.SampleValue, stepVal model.SampleValue, startTime time.Time) (resultValues model.Values) { + currentTime := startTime for currentVal := startVal; currentVal <= endVal; currentVal += stepVal { sample := model.SamplePair{ Value: currentVal, @@ -74,7 +72,7 @@ var testMatrix = ast.Matrix{ "instance": "0", "group": "production", }, - Values: getTestValueStream(0, 100, 10), + Values: getTestValueStream(0, 100, 10, testStartTime), }, { Metric: model.Metric{ @@ -83,7 +81,7 @@ var testMatrix = ast.Matrix{ "instance": "1", "group": "production", }, - Values: getTestValueStream(0, 200, 20), + Values: getTestValueStream(0, 200, 20, testStartTime), }, { Metric: model.Metric{ @@ -92,7 +90,7 @@ var testMatrix = ast.Matrix{ "instance": "0", "group": "canary", }, - Values: getTestValueStream(0, 300, 30), + Values: getTestValueStream(0, 300, 30, testStartTime), }, { Metric: model.Metric{ @@ -101,7 +99,7 @@ var testMatrix = ast.Matrix{ "instance": "1", "group": "canary", }, - Values: getTestValueStream(0, 400, 40), + Values: getTestValueStream(0, 400, 40, testStartTime), }, { Metric: model.Metric{ @@ -110,7 +108,7 @@ var testMatrix = ast.Matrix{ "instance": "0", "group": "production", }, - Values: getTestValueStream(0, 500, 50), + Values: getTestValueStream(0, 500, 50, testStartTime), }, { Metric: model.Metric{ @@ -119,7 +117,7 @@ var testMatrix = ast.Matrix{ "instance": "1", "group": "production", }, - Values: getTestValueStream(0, 600, 60), + Values: getTestValueStream(0, 600, 60, testStartTime), }, { Metric: model.Metric{ @@ -128,7 +126,7 @@ var testMatrix = ast.Matrix{ "instance": "0", "group": "canary", }, - Values: getTestValueStream(0, 700, 70), + Values: getTestValueStream(0, 700, 70, testStartTime), }, { Metric: model.Metric{ @@ -137,7 +135,7 @@ var testMatrix = ast.Matrix{ "instance": "1", "group": "canary", }, - Values: getTestValueStream(0, 800, 80), + Values: getTestValueStream(0, 800, 80, testStartTime), }, // Single-letter metric and label names. { @@ -145,7 +143,14 @@ var testMatrix = ast.Matrix{ model.MetricNameLabel: "x", "y": "testvalue", }, - Values: getTestValueStream(0, 100, 10), + Values: getTestValueStream(0, 100, 10, testStartTime), + }, + // Counter resets. + { + Metric: model.Metric{ + model.MetricNameLabel: "testcounter", + }, + Values: append(getTestValueStream(0, 40, 10, testStartTime), getTestValueStream(0, 50, 10, testStartTime.Add(testSampleInterval*5))...), }, } diff --git a/rules/rules_test.go b/rules/rules_test.go index 601a41acf..0d3092adf 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -318,6 +318,18 @@ func TestExpressions(t *testing.T) { output: []string{"http_requests{group='canary',instance='1',job='app-server'} => 0.26666666666666666 @[%v]"}, fullRanges: 1, intervalRanges: 0, + }, { + // Counter resets are ignored by delta() if counter == 1. + expr: "delta(testcounter[50m], 1)", + output: []string{"testcounter{} => 90 @[%v]"}, + fullRanges: 1, + intervalRanges: 0, + }, { + // Counter resets are not ignored by delta() if counter == 0. + expr: "delta(testcounter[50m], 0)", + output: []string{"testcounter{} => 50 @[%v]"}, + fullRanges: 1, + intervalRanges: 0, }, { // Empty expressions shouldn't parse. expr: "", diff --git a/storage/metric/interface.go b/storage/metric/interface.go index 336a31eb4..70eda577c 100644 --- a/storage/metric/interface.go +++ b/storage/metric/interface.go @@ -47,8 +47,12 @@ type MetricPersistence interface { // Get the metric associated with the provided fingerprint. GetMetricForFingerprint(*model.Fingerprint) (model.Metric, error) + // Get the two metric values that are immediately adjacent to a given time. GetValueAtTime(*model.Fingerprint, time.Time) model.Values - GetBoundaryValues(*model.Fingerprint, model.Interval) (first model.Values, second model.Values) + // Get the boundary values of an interval: the first value older than the + // interval start, and the first value younger than the interval end. + GetBoundaryValues(*model.Fingerprint, model.Interval) model.Values + // Get all values contained within a provided interval. GetRangeValues(*model.Fingerprint, model.Interval) model.Values // Get all label values that are associated with a given label name. GetAllValuesForLabel(model.LabelName) (model.LabelValues, error) @@ -58,11 +62,11 @@ type MetricPersistence interface { // MakeView(builder ViewRequestBuilder, deadline time.Duration) (View, error) } -// View provides view of the values in the datastore subject to the request of a -// preloading operation. +// View provides a view of the values in the datastore subject to the request +// of a preloading operation. type View interface { GetValueAtTime(*model.Fingerprint, time.Time) model.Values - GetBoundaryValues(*model.Fingerprint, model.Interval) (first model.Values, second model.Values) + GetBoundaryValues(*model.Fingerprint, model.Interval) model.Values GetRangeValues(*model.Fingerprint, model.Interval) model.Values // Destroy this view. diff --git a/storage/metric/leveldb.go b/storage/metric/leveldb.go index 7e281169a..9d374f93c 100644 --- a/storage/metric/leveldb.go +++ b/storage/metric/leveldb.go @@ -793,15 +793,15 @@ func (l *LevelDBMetricPersistence) GetMetricForFingerprint(f *model.Fingerprint) return } -func (l LevelDBMetricPersistence) GetValueAtTime(f *model.Fingerprint, t time.Time) (samples model.Values) { +func (l LevelDBMetricPersistence) GetValueAtTime(f *model.Fingerprint, t time.Time) model.Values { panic("Not implemented") } -func (l LevelDBMetricPersistence) GetBoundaryValues(f *model.Fingerprint, i model.Interval) (first model.Values, second model.Values) { +func (l LevelDBMetricPersistence) GetBoundaryValues(f *model.Fingerprint, i model.Interval) model.Values { panic("Not implemented") } -func (l *LevelDBMetricPersistence) GetRangeValues(f *model.Fingerprint, i model.Interval) (samples model.Values) { +func (l *LevelDBMetricPersistence) GetRangeValues(f *model.Fingerprint, i model.Interval) model.Values { panic("Not implemented") } diff --git a/storage/metric/memory.go b/storage/metric/memory.go index 8fa1a9ccb..3dea6946e 100644 --- a/storage/metric/memory.go +++ b/storage/metric/memory.go @@ -102,8 +102,27 @@ func (s *stream) getValueAtTime(t time.Time) model.Values { } } -func (s *stream) getBoundaryValues(i model.Interval) (model.Values, model.Values) { - return s.getValueAtTime(i.OldestInclusive), s.getValueAtTime(i.NewestInclusive) +func (s *stream) getBoundaryValues(in model.Interval) model.Values { + s.RLock() + defer s.RUnlock() + + oldest := sort.Search(len(s.values), func(i int) bool { + return !s.values[i].Timestamp.Before(in.OldestInclusive) + }) + + newest := sort.Search(len(s.values), func(i int) bool { + return s.values[i].Timestamp.After(in.NewestInclusive) + }) + + resultRange := s.values[oldest:newest] + switch len(resultRange) { + case 0: + return model.Values{} + case 1: + return model.Values{resultRange[0]} + default: + return model.Values{resultRange[0], resultRange[len(resultRange)-1]} + } } func (s *stream) getRangeValues(in model.Interval) model.Values { @@ -286,12 +305,12 @@ func (s *memorySeriesStorage) GetValueAtTime(f *model.Fingerprint, t time.Time) return series.getValueAtTime(t) } -func (s *memorySeriesStorage) GetBoundaryValues(f *model.Fingerprint, i model.Interval) (model.Values, model.Values) { +func (s *memorySeriesStorage) GetBoundaryValues(f *model.Fingerprint, i model.Interval) model.Values { s.RLock() series, ok := s.fingerprintToSeries[*f] s.RUnlock() if !ok { - return nil, nil + return nil } return series.getBoundaryValues(i) diff --git a/storage/metric/rule_integration_test.go b/storage/metric/rule_integration_test.go index ab01995cd..bd8655756 100644 --- a/storage/metric/rule_integration_test.go +++ b/storage/metric/rule_integration_test.go @@ -352,321 +352,7 @@ func GetValueAtTimeTests(persistenceMaker func() (MetricPersistence, test.Closer } } -func GetBoundaryValuesTests(persistenceMaker func() (MetricPersistence, test.Closer), t test.Tester) { - type value struct { - year int - month time.Month - day int - hour int - value model.SampleValue - } - - type input struct { - openYear int - openMonth time.Month - openDay int - openHour int - endYear int - endMonth time.Month - endDay int - endHour int - } - - type output struct { - open []model.SampleValue - end []model.SampleValue - } - - type behavior struct { - name string - input input - output output - } - - var contexts = []struct { - name string - values []value - behaviors []behavior - }{ - { - name: "no values", - values: []value{}, - behaviors: []behavior{ - { - name: "non-existent interval", - input: input{ - openYear: 1984, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1985, - endMonth: 3, - endDay: 30, - endHour: 0, - }, - }, - }, - }, - { - name: "single value", - values: []value{ - { - year: 1984, - month: 3, - day: 30, - hour: 0, - value: 0, - }, - }, - behaviors: []behavior{ - { - name: "on start but missing end", - input: input{ - openYear: 1984, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1985, - endMonth: 3, - endDay: 30, - endHour: 0, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{0}, - }, - }, - { - name: "non-existent interval after", - input: input{ - openYear: 1984, - openMonth: 3, - openDay: 31, - openHour: 0, - endYear: 1985, - endMonth: 3, - endDay: 30, - endHour: 0, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{0}, - }, - }, - { - name: "non-existent interval before", - input: input{ - openYear: 1983, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1984, - endMonth: 3, - endDay: 29, - endHour: 0, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{0}, - }, - }, - { - name: "on end but not start", - input: input{ - openYear: 1983, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1984, - endMonth: 3, - endDay: 30, - endHour: 0, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{0}, - }, - }, - { - name: "before point", - input: input{ - openYear: 1982, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1983, - endMonth: 3, - endDay: 30, - endHour: 0, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{0}, - }, - }, - { - name: "after point", - input: input{ - openYear: 1985, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1986, - endMonth: 3, - endDay: 30, - endHour: 0, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{0}, - }, - }, - { - name: "spanning point", - input: input{ - openYear: 1983, - openMonth: 9, - openDay: 29, - openHour: 12, - endYear: 1984, - endMonth: 9, - endDay: 28, - endHour: 12, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{0}, - }, - }, - }, - }, - { - name: "double values", - values: []value{ - { - year: 1984, - month: 3, - day: 30, - hour: 0, - value: 0, - }, - { - year: 1985, - month: 3, - day: 30, - hour: 0, - value: 1, - }, - }, - behaviors: []behavior{ - { - name: "on points", - input: input{ - openYear: 1984, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1985, - endMonth: 3, - endDay: 30, - endHour: 0, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{1}, - }, - }, - { - name: "on first before second", - input: input{ - openYear: 1984, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1984, - endMonth: 6, - endDay: 29, - endHour: 6, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{0, 1}, - }, - }, - { - name: "on first after second", - input: input{ - openYear: 1984, - openMonth: 3, - openDay: 30, - openHour: 0, - endYear: 1985, - endMonth: 6, - endDay: 29, - endHour: 6, - }, - output: output{ - open: []model.SampleValue{0}, - end: []model.SampleValue{1}, - }, - }, - }, - }, - } - - for i, context := range contexts { - // Wrapping in function to enable garbage collection of resources. - func() { - p, closer := persistenceMaker() - - defer closer.Close() - defer p.Close() - - m := model.Metric{ - model.MetricNameLabel: "age_in_years", - } - - for _, value := range context.values { - testAppendSample(p, model.Sample{ - Value: model.SampleValue(value.value), - Timestamp: time.Date(value.year, value.month, value.day, value.hour, 0, 0, 0, time.UTC), - Metric: m, - }, t) - } - - for j, behavior := range context.behaviors { - input := behavior.input - open := time.Date(input.openYear, input.openMonth, input.openDay, input.openHour, 0, 0, 0, time.UTC) - end := time.Date(input.endYear, input.endMonth, input.endDay, input.endHour, 0, 0, 0, time.UTC) - interval := model.Interval{ - OldestInclusive: open, - NewestInclusive: end, - } - openValues, endValues := p.GetBoundaryValues(model.NewFingerprintFromMetric(m), interval) - if len(behavior.output.open) != len(openValues) { - t.Fatalf("%d.%d(%s). Expected %d open values but got: %q\n", i, j, behavior.name, len(behavior.output.open), openValues) - } - if len(behavior.output.end) != len(endValues) { - t.Fatalf("%d.%d(%s). Expected %d open values but got: %q\n", i, j, behavior.name, len(behavior.output.open), openValues) - } - - for k, samplePair := range openValues { - if samplePair.Value != behavior.output.open[k] { - t.Fatalf("%d.%d.%d(%s). Expected open to be %v but got %v\n", i, j, k, behavior.name, behavior.output.open[k], samplePair.Value) - } - } - - for k, samplePair := range endValues { - if samplePair.Value != behavior.output.end[k] { - t.Fatalf("%d.%d.%d(%s). Expected end to be %v but got %v\n", i, j, k, behavior.name, behavior.output.end[k], samplePair.Value) - } - } - } - }() - } -} - -func GetRangeValuesTests(persistenceMaker func() (MetricPersistence, test.Closer), t test.Tester) { +func GetRangeValuesTests(persistenceMaker func() (MetricPersistence, test.Closer), onlyBoundaries bool, t test.Tester) { type value struct { year int month time.Month @@ -949,6 +635,172 @@ func GetRangeValuesTests(persistenceMaker func() (MetricPersistence, test.Closer }, }, }, + { + name: "three values", + values: []value{ + { + year: 1984, + month: 3, + day: 30, + hour: 0, + value: 0, + }, + { + year: 1985, + month: 3, + day: 30, + hour: 0, + value: 1, + }, + { + year: 1986, + month: 3, + day: 30, + hour: 0, + value: 2, + }, + }, + behaviors: []behavior{ + { + name: "start on first value", + input: input{ + openYear: 1984, + openMonth: 3, + openDay: 30, + openHour: 0, + endYear: 1985, + endMonth: 3, + endDay: 30, + endHour: 0, + }, + output: []output{ + { + year: 1984, + month: 3, + day: 30, + hour: 0, + value: 0, + }, + { + year: 1985, + month: 3, + day: 30, + hour: 0, + value: 1, + }, + }, + }, + { + name: "start on second value", + input: input{ + openYear: 1985, + openMonth: 3, + openDay: 30, + openHour: 0, + endYear: 1986, + endMonth: 3, + endDay: 30, + endHour: 0, + }, + output: []output{ + { + year: 1985, + month: 3, + day: 30, + hour: 0, + value: 1, + }, + { + year: 1986, + month: 3, + day: 30, + hour: 0, + value: 2, + }, + }, + }, + { + name: "end on first value", + input: input{ + openYear: 1983, + openMonth: 3, + openDay: 30, + openHour: 0, + endYear: 1984, + endMonth: 3, + endDay: 30, + endHour: 0, + }, + output: []output{ + { + year: 1984, + month: 3, + day: 30, + hour: 0, + value: 0, + }, + }, + }, + { + name: "end on second value", + input: input{ + openYear: 1985, + openMonth: 1, + openDay: 1, + openHour: 0, + endYear: 1985, + endMonth: 3, + endDay: 30, + endHour: 0, + }, + output: []output{ + { + year: 1985, + month: 3, + day: 30, + hour: 0, + value: 1, + }, + }, + }, + { + name: "overlap on values", + input: input{ + openYear: 1983, + openMonth: 3, + openDay: 30, + openHour: 0, + endYear: 1986, + endMonth: 3, + endDay: 30, + endHour: 0, + }, + output: []output{ + { + year: 1984, + month: 3, + day: 30, + hour: 0, + value: 0, + }, + { + year: 1985, + month: 3, + day: 30, + hour: 0, + value: 1, + }, + { + year: 1986, + month: 3, + day: 30, + hour: 0, + value: 2, + }, + }, + }, + }, + }, } for i, context := range contexts { @@ -980,26 +832,41 @@ func GetRangeValuesTests(persistenceMaker func() (MetricPersistence, test.Closer NewestInclusive: end, } - values := p.GetRangeValues(model.NewFingerprintFromMetric(m), in) - - if values == nil && len(behavior.output) != 0 { - t.Fatalf("%d.%d(%s). Expected %s but got: %s\n", i, j, behavior.name, behavior.output, values) - } - - if behavior.output == nil { - if values != nil { - t.Fatalf("%d.%d(%s). Expected nil values but got: %s\n", i, j, behavior.name, values) + actualValues := model.Values{} + expectedValues := []output{} + fp := model.NewFingerprintFromMetric(m) + if onlyBoundaries { + actualValues = p.GetBoundaryValues(fp, in) + l := len(behavior.output) + if l == 1 { + expectedValues = behavior.output[0:1] + } + if l > 1 { + expectedValues = append(behavior.output[0:1], behavior.output[l-1]) } } else { - if len(behavior.output) != len(values) { - t.Fatalf("%d.%d(%s). Expected length %d but got: %d\n", i, j, behavior.name, len(behavior.output), len(values)) + actualValues = p.GetRangeValues(fp, in) + expectedValues = behavior.output + } + + if actualValues == nil && len(expectedValues) != 0 { + t.Fatalf("%d.%d(%s). Expected %s but got: %s\n", i, j, behavior.name, expectedValues, actualValues) + } + + if expectedValues == nil { + if actualValues != nil { + t.Fatalf("%d.%d(%s). Expected nil values but got: %s\n", i, j, behavior.name, actualValues) + } + } else { + if len(expectedValues) != len(actualValues) { + t.Fatalf("%d.%d(%s). Expected length %d but got: %d\n", i, j, behavior.name, len(expectedValues), len(actualValues)) } - for k, actual := range values { - expected := behavior.output[k] + for k, actual := range actualValues { + expected := expectedValues[k] if actual.Value != model.SampleValue(expected.value) { - t.Fatalf("%d.%d.%d(%s). Expected %d but got: %d\n", i, j, k, behavior.name, expected.value, actual.Value) + t.Fatalf("%d.%d.%d(%s). Expected %v but got: %v\n", i, j, k, behavior.name, expected.value, actual.Value) } if actual.Timestamp.Year() != expected.year { @@ -1045,14 +912,6 @@ func BenchmarkMemoryGetValueAtTime(b *testing.B) { } } -func testMemoryGetBoundaryValues(t test.Tester) { - persistenceMaker := func() (MetricPersistence, test.Closer) { - return NewMemorySeriesStorage(), test.NilCloser - } - - GetBoundaryValuesTests(persistenceMaker, t) -} - func TestMemoryGetBoundaryValues(t *testing.T) { testMemoryGetBoundaryValues(t) } @@ -1068,7 +927,15 @@ func testMemoryGetRangeValues(t test.Tester) { return NewMemorySeriesStorage(), test.NilCloser } - GetRangeValuesTests(persistenceMaker, t) + GetRangeValuesTests(persistenceMaker, false, t) +} + +func testMemoryGetBoundaryValues(t test.Tester) { + persistenceMaker := func() (MetricPersistence, test.Closer) { + return NewMemorySeriesStorage(), test.NilCloser + } + + GetRangeValuesTests(persistenceMaker, true, t) } func TestMemoryGetRangeValues(t *testing.T) {