Skip to content

Commit 448cc74

Browse files
authored
fix: Restrict start/end timestamp to requested range for scheduler (#20086)
1 parent 397da27 commit 448cc74

File tree

3 files changed

+28
-14
lines changed

3 files changed

+28
-14
lines changed

pkg/querier/queryrange/engine_router.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ func (e *engineRouter) splitOverlapping(r queryrangebase.Request, v2Start, v2End
170170
}
171171

172172
// align the ranges by step before splitting.
173-
start, end := alignStartEnd(stepNs, r.GetStart(), r.GetEnd())
173+
start, end := r.GetStart(), r.GetEnd()
174174
v2Start, v2End = alignStartEnd(stepNs, v2Start, v2End)
175175

176176
// chunk req before V2 engine range

pkg/querier/queryrange/engine_router_test.go

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package queryrange
33
import (
44
"context"
55
"fmt"
6+
"sort"
67
"testing"
78
"time"
89

@@ -118,9 +119,16 @@ func TestEngineRouter_split(t *testing.T) {
118119
splitter := &engineRouter{forMetricQuery: tt.forMetricQuery}
119120
splits := splitter.splitOverlapping(baseReq.WithStartEnd(tt.start, tt.end), v2Start, v2End)
120121

121-
var (
122-
gotV1 []queryrangebase.Request
123-
)
122+
// Sort the splits by start time to ensure the splits are in the correct order.
123+
sort.Slice(splits, func(i, j int) bool {
124+
return splits[i].req.GetStart().Before(splits[j].req.GetStart())
125+
})
126+
127+
// Check the splits respect the original query's start and end times.
128+
require.Equal(t, tt.start, splits[0].req.GetStart())
129+
require.Equal(t, tt.end, splits[len(splits)-1].req.GetEnd())
130+
131+
var gotV1 []queryrangebase.Request
124132
for _, split := range splits {
125133
if split.isV2Engine {
126134
if tt.expectedV2Req != nil {
@@ -170,13 +178,13 @@ func TestEngineRouter_stepAlignment(t *testing.T) {
170178
forMetricQuery: true,
171179
expectedV1Reqs: []queryrangebase.Request{
172180
buildReq(
173-
now.Add(-3*24*time.Hour).Truncate(time.Second), // start of query is rounded down
181+
now.Add(-3*24*time.Hour),
174182
now.Add(-2*24*time.Hour).Truncate(time.Second).Add(-time.Second), // v2 start rounded down, minus step gap
175183
1000,
176184
),
177185
buildReq(
178186
now.Add(-2*time.Hour).Truncate(time.Second).Add(time.Second), // v2 end is rounded up
179-
now.Add(-time.Hour).Truncate(time.Second).Add(time.Second), // end is rounded up
187+
now.Add(-time.Hour),
180188
1000,
181189
),
182190
},
@@ -192,13 +200,13 @@ func TestEngineRouter_stepAlignment(t *testing.T) {
192200
forMetricQuery: false, // no gaps between splits for log queries
193201
expectedV1Reqs: []queryrangebase.Request{
194202
buildReq(
195-
now.Add(-3*24*time.Hour).Truncate(time.Second), // start of query is rounded down
203+
now.Add(-3*24*time.Hour),
196204
now.Add(-2*24*time.Hour).Truncate(time.Second), // v2 start rounded down, no step gap
197205
1000,
198206
),
199207
buildReq(
200208
now.Add(-2*time.Hour).Truncate(time.Second).Add(time.Second), // v2 end is rounded up
201-
now.Add(-time.Hour).Truncate(time.Second).Add(time.Second), // end is rounded up
209+
now.Add(-time.Hour),
202210
1000,
203211
),
204212
},
@@ -214,13 +222,13 @@ func TestEngineRouter_stepAlignment(t *testing.T) {
214222
forMetricQuery: true,
215223
expectedV1Reqs: []queryrangebase.Request{
216224
buildReq(
217-
now.Add(-3*24*time.Hour).Truncate(3*time.Second),
225+
now.Add(-3*24*time.Hour),
218226
now.Add(-2*24*time.Hour).Truncate(3*time.Second).Add(-3*time.Second), // rounded down, minus step gap
219227
3000,
220228
),
221229
buildReq(
222230
now.Add(-2*time.Hour).Truncate(3*time.Second).Add(3*time.Second),
223-
now.Add(-time.Hour).Truncate(3*time.Second).Add(3*time.Second),
231+
now.Add(-time.Hour),
224232
3000,
225233
),
226234
},
@@ -237,9 +245,15 @@ func TestEngineRouter_stepAlignment(t *testing.T) {
237245
splitter := &engineRouter{forMetricQuery: tt.forMetricQuery}
238246
splits := splitter.splitOverlapping(tt.req, v2Start, v2End)
239247

240-
var (
241-
gotV1 []queryrangebase.Request
242-
)
248+
sort.Slice(splits, func(i, j int) bool {
249+
return splits[i].req.GetStart().Before(splits[j].req.GetStart())
250+
})
251+
252+
// Check the splits respect the original query's start and end times.
253+
require.Equal(t, tt.req.GetStart(), splits[0].req.GetStart())
254+
require.Equal(t, tt.req.GetEnd(), splits[len(splits)-1].req.GetEnd())
255+
256+
var gotV1 []queryrangebase.Request
243257
for _, split := range splits {
244258
if split.isV2Engine {
245259
if tt.expectedV2Req != nil {

pkg/querier/queryrange/roundtrip.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ func NewLogFilterTripperware(cfg Config, engineOpts logql.EngineOpts, routerConf
732732

733733
// route query range supported by v2 engine to the new engine handler.
734734
if routerConfig.Enabled {
735-
engineRouterMiddleware := newEngineRouterMiddleware(routerConfig, chunksEngineMWs, merger, true, log)
735+
engineRouterMiddleware := newEngineRouterMiddleware(routerConfig, chunksEngineMWs, merger, false, log)
736736
queryRangeMiddleware = append(
737737
queryRangeMiddleware,
738738
base.InstrumentMiddleware("v2_engine_router", metrics.InstrumentMiddlewareMetrics),

0 commit comments

Comments
 (0)