chore: [k205] fix: Do not filter out chunks for store when `From==Thr…

…ough` and `From==start` (#13121) Backport d9cc513 from #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: * Insert a single log line with a timestamp exactly at point of an hour * Flush ingester * Query log line with a split interval of 1h Co-authored-by: Christian Haudum <[email protected]>
grafana · Jun 4, 2024 · 6e66e68 · 6e66e68
1 parent 00d3c7a
commit 6e66e68
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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)