From d9cc513fd2decf96d047d388136417c03ccdc682 Mon Sep 17 00:00:00 2001 From: Christian Haudum Date: Tue, 4 Jun 2024 18:38:07 +0200 Subject: [PATCH] fix: Do not filter out chunks for store when `From==Through` and `From==start` (#13117) This PR fixes a bug where chunks are incorrectly filtered out when their `From` timestamp is equal to their `Through` timestamp (which happens when there is a single log line) and the `From` timestamp is equal to the `from` time of the of the request. How to reproduce: 1. Insert a single log line with a timestamp exactly at point of an hour 2. Flush ingester 3. Query log line with a split interval of 1h Signed-off-by: Christian Haudum Co-authored-by: Ed Welch --- pkg/storage/stores/composite_store.go | 4 ++-- pkg/storage/stores/composite_store_entry.go | 5 ++++- pkg/storage/stores/composite_store_test.go | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/pkg/storage/stores/composite_store.go b/pkg/storage/stores/composite_store.go index b975873c3043..834d9602727f 100644 --- a/pkg/storage/stores/composite_store.go +++ b/pkg/storage/stores/composite_store.go @@ -172,8 +172,8 @@ func (c CompositeStore) GetChunks( ) ([][]chunk.Chunk, []*fetcher.Fetcher, error) { chunkIDs := [][]chunk.Chunk{} fetchers := []*fetcher.Fetcher{} - err := c.forStores(ctx, from, through, func(innerCtx context.Context, from, through model.Time, store Store) error { - ids, fetcher, err := store.GetChunks(innerCtx, userID, from, through, predicate, storeChunksOverride) + err := c.forStores(ctx, from, through, func(innerCtx context.Context, innerFrom, innerThrough model.Time, store Store) error { + ids, fetcher, err := store.GetChunks(innerCtx, userID, innerFrom, innerThrough, predicate, storeChunksOverride) if err != nil { return err } diff --git a/pkg/storage/stores/composite_store_entry.go b/pkg/storage/stores/composite_store_entry.go index 2c6e3c5e4a90..5b8db237f4c3 100644 --- a/pkg/storage/stores/composite_store_entry.go +++ b/pkg/storage/stores/composite_store_entry.go @@ -90,7 +90,10 @@ func (c *storeEntry) GetChunks( func filterForTimeRange(refs []*logproto.ChunkRef, from, through model.Time) []chunk.Chunk { filtered := make([]chunk.Chunk, 0, len(refs)) for _, ref := range refs { - if through >= ref.From && from < ref.Through { + // Only include chunks where the query start time (from) is < the chunk end time (ref.Through) + // and the query end time (through) is >= the chunk start time (ref.From) + // A special case also exists where a chunk can contain a single log line which results in ref.From being equal to ref.Through, and that is equal to the from time. + if (through >= ref.From && from < ref.Through) || (ref.From == from && ref.Through == from) { filtered = append(filtered, chunk.Chunk{ ChunkRef: *ref, }) diff --git a/pkg/storage/stores/composite_store_test.go b/pkg/storage/stores/composite_store_test.go index ab44f6b69d3c..28052c528f7c 100644 --- a/pkg/storage/stores/composite_store_test.go +++ b/pkg/storage/stores/composite_store_test.go @@ -421,6 +421,23 @@ func TestFilterForTimeRange(t *testing.T) { through: 7, exp: mkChks(5, 7), }, + { + desc: "ref with from == through", + input: []*logproto.ChunkRef{ + {From: 1, Through: 1}, // outside + {From: 2, Through: 2}, // ref.From == from == ref.Through + {From: 3, Through: 3}, // inside + {From: 4, Through: 4}, // ref.From == through == ref.Through + {From: 5, Through: 5}, // outside + }, + from: 2, + through: 4, + exp: []chunk.Chunk{ + {ChunkRef: logproto.ChunkRef{From: 2, Through: 2}}, + {ChunkRef: logproto.ChunkRef{From: 3, Through: 3}}, + {ChunkRef: logproto.ChunkRef{From: 4, Through: 4}}, + }, + }, } { t.Run(tc.desc, func(t *testing.T) { got := filterForTimeRange(tc.input, tc.from, tc.through)