From 6e89250a5d937485a140c6ba6dcdb35d2db51cd0 Mon Sep 17 00:00:00 2001
From: Bryan Boreham <bjboreham@gmail.com>
Date: Fri, 26 Jul 2024 09:49:25 +0100
Subject: [PATCH] Revert "Chunked remote read: close the querier earlier"

Believed to trigger segmentation faults due to memory-mapped block
data still being accessed by iterators after the querier is closed.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
---
 storage/remote/read_handler.go | 53 ++++++++++++++--------------------
 1 file changed, 21 insertions(+), 32 deletions(-)

diff --git a/storage/remote/read_handler.go b/storage/remote/read_handler.go
index 2a00ce897f..ffc64c9c3f 100644
--- a/storage/remote/read_handler.go
+++ b/storage/remote/read_handler.go
@@ -202,16 +202,34 @@ func (h *readHandler) remoteReadStreamedXORChunks(ctx context.Context, w http.Re
 				return err
 			}
 
-			chunks := h.getChunkSeriesSet(ctx, query, filteredMatchers)
-			if err := chunks.Err(); err != nil {
+			querier, err := h.queryable.ChunkQuerier(query.StartTimestampMs, query.EndTimestampMs)
+			if err != nil {
 				return err
 			}
+			defer func() {
+				if err := querier.Close(); err != nil {
+					level.Warn(h.logger).Log("msg", "Error on chunk querier close", "err", err.Error())
+				}
+			}()
+
+			var hints *storage.SelectHints
+			if query.Hints != nil {
+				hints = &storage.SelectHints{
+					Start:    query.Hints.StartMs,
+					End:      query.Hints.EndMs,
+					Step:     query.Hints.StepMs,
+					Func:     query.Hints.Func,
+					Grouping: query.Hints.Grouping,
+					Range:    query.Hints.RangeMs,
+					By:       query.Hints.By,
+				}
+			}
 
 			ws, err := StreamChunkedReadResponses(
 				NewChunkedWriter(w, f),
 				int64(i),
 				// The streaming API has to provide the series sorted.
-				chunks,
+				querier.Select(ctx, true, hints, filteredMatchers...),
 				sortedExternalLabels,
 				h.remoteReadMaxBytesInFrame,
 				h.marshalPool,
@@ -236,35 +254,6 @@ func (h *readHandler) remoteReadStreamedXORChunks(ctx context.Context, w http.Re
 	}
 }
 
-// getChunkSeriesSet executes a query to retrieve a ChunkSeriesSet,
-// encapsulating the operation in its own function to ensure timely release of
-// the querier resources.
-func (h *readHandler) getChunkSeriesSet(ctx context.Context, query *prompb.Query, filteredMatchers []*labels.Matcher) storage.ChunkSeriesSet {
-	querier, err := h.queryable.ChunkQuerier(query.StartTimestampMs, query.EndTimestampMs)
-	if err != nil {
-		return storage.ErrChunkSeriesSet(err)
-	}
-	defer func() {
-		if err := querier.Close(); err != nil {
-			level.Warn(h.logger).Log("msg", "Error on chunk querier close", "err", err.Error())
-		}
-	}()
-
-	var hints *storage.SelectHints
-	if query.Hints != nil {
-		hints = &storage.SelectHints{
-			Start:    query.Hints.StartMs,
-			End:      query.Hints.EndMs,
-			Step:     query.Hints.StepMs,
-			Func:     query.Hints.Func,
-			Grouping: query.Hints.Grouping,
-			Range:    query.Hints.RangeMs,
-			By:       query.Hints.By,
-		}
-	}
-	return querier.Select(ctx, true, hints, filteredMatchers...)
-}
-
 // filterExtLabelsFromMatchers change equality matchers which match external labels
 // to a matcher that looks for an empty label,
 // as that label should not be present in the storage.