From 67e791c1edfe90150521423a61ce80da0a4409b5 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Fri, 14 Jul 2023 18:28:56 +0200 Subject: [PATCH 1/2] store-gateway: wait for query gate after loading blocks A surge in lazy loading index headers can saturate the query concurrency limit and effectively block any other queries from even starting. This was evident in a recent similar incident where the request rate on the full query path was impacted by store-gateways lazy-loading blocks. Below are screenshots of request rate, latency, lazy loading latency and query gate latency reported by store-gateways. We already have a concurrency gate for lazy loading index headers added by Heather, which prevents from overloading the disk. If we decouple the query gate and the header loading gate, queries that require loading many index headers will be waiting on a separate gate and not impact queries which are querying already loaded blocks. #### Note to reviewers This is a draft PR because I suspect it will conflict with the changes in stream-store, so I will rebase it after merging that PR first. Signed-off-by: Dimitar Dimitrov --- pkg/storegateway/bucket.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/storegateway/bucket.go b/pkg/storegateway/bucket.go index de52bcbc08a..7bc8dcb4cbe 100644 --- a/pkg/storegateway/bucket.go +++ b/pkg/storegateway/bucket.go @@ -556,17 +556,6 @@ func (s *BucketStore) Series(req *storepb.SeriesRequest, srv storepb.Store_Serie err = status.Error(code, err.Error()) }() - if s.queryGate != nil { - tracing.DoWithSpan(srv.Context(), "store_query_gate_ismyturn", func(ctx context.Context, _ tracing.Span) { - err = s.queryGate.Start(srv.Context()) - }) - if err != nil { - return errors.Wrapf(err, "failed to wait for turn") - } - - defer s.queryGate.Done() - } - matchers, err := storepb.MatchersToPromMatchers(req.Matchers...) if err != nil { return status.Error(codes.InvalidArgument, err.Error()) @@ -615,6 +604,16 @@ func (s *BucketStore) Series(req *storepb.SeriesRequest, srv storepb.Store_Serie readers = newChunkReaders(chunkReaders) } + // Wait for the query gate only after opening blocks. Opening blocks is usually fast (~1ms), + // but sometimes it can take minutes if the block isn't loaded and there is a surge in queries for unloaded blocks. + tracing.DoWithSpan(ctx, "store_query_gate_ismyturn", func(ctx context.Context, _ tracing.Span) { + err = s.queryGate.Start(ctx) + }) + if err != nil { + return errors.Wrapf(err, "failed to wait for turn") + } + defer s.queryGate.Done() + seriesSet, resHints, err := s.streamingSeriesSetForBlocks(ctx, req, blocks, indexReaders, readers, shardSelector, matchers, chunksLimiter, seriesLimiter, stats) if err != nil { return err From 660a1ce290802b0097a8d0205b823bf9aadae2fc Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Tue, 18 Jul 2023 10:19:52 +0200 Subject: [PATCH 2/2] Fix linter Signed-off-by: Dimitar Dimitrov --- pkg/storegateway/bucket.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/storegateway/bucket.go b/pkg/storegateway/bucket.go index de5dd2457e2..82a540da602 100644 --- a/pkg/storegateway/bucket.go +++ b/pkg/storegateway/bucket.go @@ -609,7 +609,7 @@ func (s *BucketStore) Series(req *storepb.SeriesRequest, srv storepb.Store_Serie readers = newChunkReaders(chunkReaders) } - // Wait for the query gate only after opening blocks. Opening blocks is usually fast (~1ms), + // Wait for the query gate only after opening blocks. Opening blocks is usually fast (~1ms), // but sometimes it can take minutes if the block isn't loaded and there is a surge in queries for unloaded blocks. tracing.DoWithSpan(ctx, "store_query_gate_ismyturn", func(ctx context.Context, _ tracing.Span) { err = s.queryGate.Start(ctx) @@ -618,7 +618,7 @@ func (s *BucketStore) Series(req *storepb.SeriesRequest, srv storepb.Store_Serie return errors.Wrapf(err, "failed to wait for turn") } defer s.queryGate.Done() - + var ( // If we are streaming the series labels and chunks separately, we don't need to fetch the postings // twice. So we use these slices to re-use them. Each reuse[i] corresponds to a single block.