Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix order of the returned results from badger backend. #1939

Merged
merged 5 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions plugin/storage/badger/spanstore/read_write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,12 @@ func TestIndexSeeks(t *testing.T) {
traces := 60
spans := 3
tid := startT

traceOrder := make([]uint64, traces)

for i := 0; i < traces; i++ {
lowId := rand.Uint64()
traceOrder[i] = lowId
tid = tid.Add(time.Duration(time.Millisecond * time.Duration(i)))

for j := 0; j < spans; j++ {
Expand Down Expand Up @@ -217,7 +221,6 @@ func TestIndexSeeks(t *testing.T) {
tags["error"] = "true"
params.Tags = tags
params.DurationMin = time.Duration(1 * time.Millisecond)
// params.DurationMax = time.Duration(1 * time.Hour)
burmanm marked this conversation as resolved.
Show resolved Hide resolved
trs, err = sr.FindTraces(context.Background(), params)
assert.NoError(t, err)
assert.Equal(t, 1, len(trs))
Expand All @@ -231,23 +234,36 @@ func TestIndexSeeks(t *testing.T) {
trs, err = sr.FindTraces(context.Background(), params)
assert.NoError(t, err)
assert.Equal(t, 2, len(trs))
assert.Equal(t, traceOrder[59], trs[0].Spans[0].TraceID.Low)
assert.Equal(t, traceOrder[55], trs[1].Spans[0].TraceID.Low)
testOrder(trs)
burmanm marked this conversation as resolved.
Show resolved Hide resolved

// Check for DESC return order with duration index
params.NumTraces = 9
params.DurationMin = time.Duration(30 * time.Millisecond) // Filters one
params.DurationMax = time.Duration(50 * time.Millisecond) // Filters three
params = &spanstore.TraceQueryParameters{
StartTimeMin: startT,
StartTimeMax: startT.Add(time.Duration(time.Hour * 1)),
DurationMin: time.Duration(30 * time.Millisecond), // Filters one
DurationMax: time.Duration(50 * time.Millisecond), // Filters three
burmanm marked this conversation as resolved.
Show resolved Hide resolved
NumTraces: 9,
}
trs, err = sr.FindTraces(context.Background(), params)
assert.NoError(t, err)
assert.Equal(t, 5, len(trs))
assert.Equal(t, 9, len(trs)) // Returns 23, we limited to 9

// Check the newest items are returned
assert.Equal(t, traceOrder[50], trs[0].Spans[0].TraceID.Low)
assert.Equal(t, traceOrder[42], trs[8].Spans[0].TraceID.Low)
testOrder(trs)

// Check for DESC return order without
// Check for DESC return order without duration index, but still with limit
params.DurationMin = 0
params.DurationMax = 0
params.NumTraces = 7
trs, err = sr.FindTraces(context.Background(), params)
assert.NoError(t, err)
assert.Equal(t, 9, len(trs))
assert.Equal(t, 7, len(trs))
assert.Equal(t, traceOrder[59], trs[0].Spans[0].TraceID.Low)
assert.Equal(t, traceOrder[53], trs[6].Spans[0].TraceID.Low)
testOrder(trs)

// StartTime, endTime scan - full table scan (so technically no index seek)
Expand All @@ -264,12 +280,14 @@ func TestIndexSeeks(t *testing.T) {

// StartTime and Duration queries
params.StartTimeMax = startT.Add(time.Duration(time.Hour * 10))
params.DurationMin = time.Duration(53 * time.Millisecond) // trace 51 (max)
params.DurationMax = time.Duration(56 * time.Millisecond) // trace 56 (min)
params.DurationMin = time.Duration(53 * time.Millisecond) // trace 51 (min)
params.DurationMax = time.Duration(56 * time.Millisecond) // trace 56 (max)

trs, err = sr.FindTraces(context.Background(), params)
assert.NoError(t, err)
assert.Equal(t, 6, len(trs))
assert.Equal(t, traceOrder[56], trs[0].Spans[0].TraceID.Low)
assert.Equal(t, traceOrder[51], trs[5].Spans[0].TraceID.Low)
testOrder(trs)
})
}
Expand Down
13 changes: 9 additions & 4 deletions plugin/storage/badger/spanstore/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,17 @@ func (r *TraceReader) scanTimeRange(plan *executionPlan) ([]model.TraceID, error
})

sort.Slice(traceKeys, func(k, h int) bool {
// This sorts by timestamp to descending order
return bytes.Compare(traceKeys[k][sizeOfTraceID+1:sizeOfTraceID+1+8], traceKeys[h][sizeOfTraceID+1:sizeOfTraceID+1+8]) > 0
burmanm marked this conversation as resolved.
Show resolved Hide resolved
})

traceIDs := make([]model.TraceID, len(traceKeys))
sizeCount := len(traceKeys)
if plan.limit > 0 {
sizeCount = plan.limit
}
traceIDs := make([]model.TraceID, sizeCount)

for i := 0; i < len(traceKeys); i++ {
for i := 0; i < len(traceKeys) && i < sizeCount; i++ {
burmanm marked this conversation as resolved.
Show resolved Hide resolved
traceIDs[i] = bytesToTraceID(traceKeys[i][1 : sizeOfTraceID+1])
}

Expand Down Expand Up @@ -377,8 +382,8 @@ func (r *TraceReader) durationQueries(plan *executionPlan, query *spanstore.Trac
durMax := uint64(model.DurationAsMicroseconds(query.DurationMax))
durMin := uint64(model.DurationAsMicroseconds(query.DurationMin))

startKey := make([]byte, 9)
endKey := make([]byte, 9)
startKey := make([]byte, 1+8)
endKey := make([]byte, 1+8)

startKey[0] = durationIndexKey
endKey[0] = durationIndexKey
Expand Down
40 changes: 1 addition & 39 deletions plugin/storage/badger/spanstore/rw_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func TestDuplicateTraceIDDetection(t *testing.T) {

traces, err := rw.FindTraceIDs(context.Background(), &spanstore.TraceQueryParameters{
ServiceName: "service",
NumTraces: 128,
NumTraces: 256, // Default is 100, we want to fetch more than there should be
StartTimeMax: time.Now().Add(time.Hour),
StartTimeMin: testSpan.StartTime.Add(-1 * time.Hour),
})
Expand Down Expand Up @@ -198,41 +198,3 @@ func TestMergeJoin(t *testing.T) {
assert.Equal(2, len(merged))
assert.Equal(uint32(2), binary.BigEndian.Uint32(merged[1]))
}

/*
func TestIndexScanReturnOrder(t *testing.T) {
runWithBadger(t, func(store *badger.DB, t *testing.T) {
testSpan := createDummySpan()
cache := NewCacheStore(store, time.Duration(1*time.Hour), true)
sw := NewSpanWriter(store, cache, time.Duration(1*time.Hour), nil)
rw := NewTraceReader(store, cache)

for i := 0; i < 1000; i++ {
testSpan.TraceID = model.TraceID{
High: rand.Uint64(),
Low: uint64(rand.Int63()),
}
testSpan.SpanID = model.SpanID(rand.Uint64())
testSpan.StartTime = testSpan.StartTime.Add(time.Duration(i) * time.Millisecond)
err := sw.WriteSpan(&testSpan)
assert.NoError(t, err)
}

tqp := &spanstore.TraceQueryParameters{
ServiceName: "service",
StartTimeMax: testSpan.StartTime.Add(time.Hour),
StartTimeMin: testSpan.StartTime.Add(-1 * time.Hour),
}

indexSeeks := make([][]byte, 0, 1)
indexSeeks = serviceQueries(tqp, indexSeeks)

ids := make([][][]byte, 0, len(indexSeeks)+1)

indexResults, _ := rw.indexSeeksToTraceIDs(tqp, indexSeeks, ids)
assert.True(t, sort.SliceIsSorted(indexResults[0], func(i, j int) bool {
return bytes.Compare(indexResults[0][i], indexResults[0][j]) < 0
}))
})
}
*/