From 943c52a3a5b6bfed141b701c7db6d974ef536f5b Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Thu, 22 Jun 2023 14:22:16 -0400 Subject: [PATCH 1/4] hint Signed-off-by: Joe Elliott --- modules/frontend/searchsharding.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/frontend/searchsharding.go b/modules/frontend/searchsharding.go index 7a22747a400..68487db6b0c 100644 --- a/modules/frontend/searchsharding.go +++ b/modules/frontend/searchsharding.go @@ -318,6 +318,8 @@ func (s *searchSharder) backendRequests(ctx context.Context, tenantID string, pa return } + fmt.Println("start", start, "end", end) + // get block metadata of blocks in start, end duration blocks = s.blockMetas(int64(start), int64(end), tenantID) From d2099c9ad927c05466c35b00a70b0440f11ae6a5 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Thu, 22 Jun 2023 14:29:22 -0400 Subject: [PATCH 2/4] more info Signed-off-by: Joe Elliott --- modules/frontend/searchsharding.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/frontend/searchsharding.go b/modules/frontend/searchsharding.go index 68487db6b0c..bed4e2edd3c 100644 --- a/modules/frontend/searchsharding.go +++ b/modules/frontend/searchsharding.go @@ -318,7 +318,7 @@ func (s *searchSharder) backendRequests(ctx context.Context, tenantID string, pa return } - fmt.Println("start", start, "end", end) + fmt.Println("req", searchReq.Query, "tenant", tenantID, "start", start, time.Unix(int64(start), 0), "end", end, time.Unix(int64(end), 0)) // get block metadata of blocks in start, end duration blocks = s.blockMetas(int64(start), int64(end), tenantID) From 183da2d12695e81b43a70b89efda552b26dd960d Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Thu, 22 Jun 2023 14:52:22 -0400 Subject: [PATCH 3/4] fix and test Signed-off-by: Joe Elliott --- modules/frontend/searchsharding.go | 8 ++++---- modules/frontend/searchsharding_test.go | 8 +++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/frontend/searchsharding.go b/modules/frontend/searchsharding.go index bed4e2edd3c..9273cb21ec5 100644 --- a/modules/frontend/searchsharding.go +++ b/modules/frontend/searchsharding.go @@ -135,7 +135,7 @@ func (s searchSharder) RoundTrip(r *http.Request) (*http.Response, error) { // build request to search ingester based on query_ingesters_until config and time range // pass subCtx in requests so we can cancel and exit early - ingesterReq, err := s.ingesterRequest(subCtx, tenantID, r, searchReq) + ingesterReq, err := s.ingesterRequest(subCtx, tenantID, r, *searchReq) if err != nil { return nil, err } @@ -435,10 +435,10 @@ func pagesPerRequest(m *backend.BlockMeta, bytesPerRequest int) int { // that covers the ingesters. If nil is returned for the http.Request then there is no ingesters query. // since this function modifies searchReq.Start and End we are taking a value instead of a pointer to prevent it from // unexpectedly changing the passed searchReq. -func (s *searchSharder) ingesterRequest(ctx context.Context, tenantID string, parent *http.Request, searchReq *tempopb.SearchRequest) (*http.Request, error) { +func (s *searchSharder) ingesterRequest(ctx context.Context, tenantID string, parent *http.Request, searchReq tempopb.SearchRequest) (*http.Request, error) { // request without start or end, search only in ingester if searchReq.Start == 0 || searchReq.End == 0 { - return buildIngesterRequest(ctx, tenantID, parent, searchReq) + return buildIngesterRequest(ctx, tenantID, parent, &searchReq) } now := time.Now() @@ -465,7 +465,7 @@ func (s *searchSharder) ingesterRequest(ctx context.Context, tenantID string, pa searchReq.Start = ingesterStart searchReq.End = ingesterEnd - return buildIngesterRequest(ctx, tenantID, parent, searchReq) + return buildIngesterRequest(ctx, tenantID, parent, &searchReq) } func buildIngesterRequest(ctx context.Context, tenantID string, parent *http.Request, searchReq *tempopb.SearchRequest) (*http.Request, error) { diff --git a/modules/frontend/searchsharding_test.go b/modules/frontend/searchsharding_test.go index c69636c7f82..3d404492b38 100644 --- a/modules/frontend/searchsharding_test.go +++ b/modules/frontend/searchsharding_test.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/http/httptest" + "reflect" "strconv" "strings" "sync" @@ -388,7 +389,8 @@ func TestIngesterRequest(t *testing.T) { searchReq, err := api.ParseSearchRequest(req) require.NoError(t, err) - actualReq, err := s.ingesterRequest(context.Background(), "test", req, searchReq) + copyReq := searchReq + actualReq, err := s.ingesterRequest(context.Background(), "test", req, *searchReq) if tc.expectedError != nil { assert.Equal(t, tc.expectedError, err) continue @@ -399,6 +401,10 @@ func TestIngesterRequest(t *testing.T) { } else { assert.Equal(t, tc.expectedURI, actualReq.RequestURI) } + + // it may seem odd to test that the searchReq is not modified, but this is to prevent an issue that + // occurs if the ingesterRequest method is changed to take a searchReq pointer + require.True(t, reflect.DeepEqual(copyReq, searchReq)) } } From 71f93150e54f76bf4dfe8e73037556412cdd67f9 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Thu, 22 Jun 2023 14:58:11 -0400 Subject: [PATCH 4/4] remove debug Signed-off-by: Joe Elliott --- modules/frontend/searchsharding.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/frontend/searchsharding.go b/modules/frontend/searchsharding.go index 9273cb21ec5..9b6a97c9256 100644 --- a/modules/frontend/searchsharding.go +++ b/modules/frontend/searchsharding.go @@ -318,8 +318,6 @@ func (s *searchSharder) backendRequests(ctx context.Context, tenantID string, pa return } - fmt.Println("req", searchReq.Query, "tenant", tenantID, "start", start, time.Unix(int64(start), 0), "end", end, time.Unix(int64(end), 0)) - // get block metadata of blocks in start, end duration blocks = s.blockMetas(int64(start), int64(end), tenantID)