Skip to content

Commit 4934cfc

Browse files
committed
Fix regression in bloom gateway that caused nothing to be filtered
Pull request #14661 added the series labels to the FilterChunkRefs request in order to be able to filter out full series that do not contain the filter key when using bloom filters. However, this chage incorrectly assumed that `Chunk.Metric` from the `GetChunks()` call contains the labels of the stream. This information is only populated once the chunk is fetched, though. Therefore the list of labels was empty, and no chunk refs were filtered. Signed-off-by: Christian Haudum <[email protected]>
1 parent 9e7d2f2 commit 4934cfc

File tree

1 file changed

+14
-5
lines changed

1 file changed

+14
-5
lines changed

pkg/indexgateway/gateway.go

+14-5
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ func buildResponses(query seriesindex.Query, batch seriesindex.ReadBatchResult,
209209

210210
func (g *Gateway) GetChunkRef(ctx context.Context, req *logproto.GetChunkRefRequest) (result *logproto.GetChunkRefResponse, err error) {
211211
logger := util_log.WithContext(ctx, g.log)
212+
sp, ctx := opentracing.StartSpanFromContext(ctx, "indexgateway.GetChunkRef")
212213

213214
instanceID, err := tenant.TenantID(ctx)
214215
if err != nil {
@@ -225,16 +226,12 @@ func (g *Gateway) GetChunkRef(ctx context.Context, req *logproto.GetChunkRefRequ
225226
return nil, err
226227
}
227228

228-
series := make(map[uint64]labels.Labels)
229229
result = &logproto.GetChunkRefResponse{
230230
Refs: make([]*logproto.ChunkRef, 0, len(chunks)),
231231
}
232232
for _, cs := range chunks {
233233
for i := range cs {
234234
result.Refs = append(result.Refs, &cs[i].ChunkRef)
235-
if _, ok := series[cs[i].Fingerprint]; !ok {
236-
series[cs[i].Fingerprint] = cs[i].Metric
237-
}
238235
}
239236
}
240237

@@ -261,10 +258,22 @@ func (g *Gateway) GetChunkRef(ctx context.Context, req *logproto.GetChunkRefRequ
261258
return result, nil
262259
}
263260

264-
chunkRefs, used, err := g.bloomQuerier.FilterChunkRefs(ctx, instanceID, req.From, req.Through, series, result.Refs, req.Plan)
261+
// Doing a "duplicate" index lookup is not ideal,
262+
// however, modifying the GetChunkRef() response, which contains the logproto.ChunkRef is neither.
263+
start := time.Now()
264+
series, err := g.indexQuerier.GetSeries(ctx, instanceID, req.From, req.Through, matchers...)
265+
seriesMap := make(map[uint64]labels.Labels, len(series))
266+
for _, s := range series {
267+
seriesMap[s.Hash()] = s
268+
}
269+
sp.LogKV("msg", "indexQuerier.GetSeries", "duration", time.Since(start), "count", len(series))
270+
271+
start = time.Now()
272+
chunkRefs, used, err := g.bloomQuerier.FilterChunkRefs(ctx, instanceID, req.From, req.Through, seriesMap, result.Refs, req.Plan)
265273
if err != nil {
266274
return nil, err
267275
}
276+
sp.LogKV("msg", "bloomQuerier.FilterChunkRefs", "duration", time.Since(start))
268277

269278
result.Refs = chunkRefs
270279
level.Info(logger).Log("msg", "return filtered chunk refs", "unfiltered", initialChunkCount, "filtered", len(result.Refs), "used_blooms", used)

0 commit comments

Comments
 (0)