From ceb66119570e874ffb1e2be07771bdc75b182da9 Mon Sep 17 00:00:00 2001 From: "Matt T. Proud" Date: Thu, 21 Mar 2013 17:09:17 +0100 Subject: [PATCH] Fix regression in subsequent range op. compactions. We have an anomaly whereby subsequent range operations fail to be compacted into one single range operation. This fixes such behavior. --- storage/metric/operation.go | 12 ++-- storage/metric/operation_test.go | 96 ++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 8 deletions(-) diff --git a/storage/metric/operation.go b/storage/metric/operation.go index 71dc929363..c111620582 100644 --- a/storage/metric/operation.go +++ b/storage/metric/operation.go @@ -431,16 +431,12 @@ func optimizeForward(pending ops) (out ops) { continue case *getValuesAlongRangeOp: // Range queries should be concatenated if they overlap. - pending = pending[1:len(pending)] - if next.GreedierThan(t) { - t.through = next.through + next.from = t.from - var ( - head = ops{t} - ) - - pending = append(head, pending...) + return optimizeForward(pending) + } else { + pending = pending[1:len(pending)] } case *getValuesAtIntervalOp: pending = pending[1:len(pending)] diff --git a/storage/metric/operation_test.go b/storage/metric/operation_test.go index eaa5824029..27741f251d 100644 --- a/storage/metric/operation_test.go +++ b/storage/metric/operation_test.go @@ -321,6 +321,46 @@ func testOptimizeTimeGroups(t test.Tester) { }, }, }, + // Regression Validation 1: Multiple Overlapping Interval Requests + // This one specific case expects no mutation. + { + in: ops{ + &getValuesAlongRangeOp{ + from: testInstant, + through: testInstant.Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(15 * time.Second), + through: testInstant.Add(15 * time.Second).Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(30 * time.Second), + through: testInstant.Add(30 * time.Second).Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(45 * time.Second), + through: testInstant.Add(45 * time.Second).Add(5 * time.Minute), + }, + }, + out: ops{ + &getValuesAlongRangeOp{ + from: testInstant, + through: testInstant.Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(15 * time.Second), + through: testInstant.Add(15 * time.Second).Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(30 * time.Second), + through: testInstant.Add(30 * time.Second).Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(45 * time.Second), + through: testInstant.Add(45 * time.Second).Add(5 * time.Minute), + }, + }, + }, } ) @@ -518,6 +558,34 @@ func testOptimizeForward(t test.Tester) { }, }, }, + // Regression Validation 1: Multiple Overlapping Interval Requests + // We expect to find compaction. + { + in: ops{ + &getValuesAlongRangeOp{ + from: testInstant, + through: testInstant.Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(15 * time.Second), + through: testInstant.Add(15 * time.Second).Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(30 * time.Second), + through: testInstant.Add(30 * time.Second).Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(45 * time.Second), + through: testInstant.Add(45 * time.Second).Add(5 * time.Minute), + }, + }, + out: ops{ + &getValuesAlongRangeOp{ + from: testInstant, + through: testInstant.Add(45 * time.Second).Add(5 * time.Minute), + }, + }, + }, } ) @@ -1007,6 +1075,34 @@ func testOptimize(t test.Tester) { }, }, }, + // Regression Validation 1: Multiple Overlapping Interval Requests + // We expect to find compaction. + { + in: ops{ + &getValuesAlongRangeOp{ + from: testInstant, + through: testInstant.Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(15 * time.Second), + through: testInstant.Add(15 * time.Second).Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(30 * time.Second), + through: testInstant.Add(30 * time.Second).Add(5 * time.Minute), + }, + &getValuesAlongRangeOp{ + from: testInstant.Add(45 * time.Second), + through: testInstant.Add(45 * time.Second).Add(5 * time.Minute), + }, + }, + out: ops{ + &getValuesAlongRangeOp{ + from: testInstant, + through: testInstant.Add(45 * time.Second).Add(5 * time.Minute), + }, + }, + }, } )