Returning 422 (instead 500) when query hit max_chunks_per_query limit with block storage (#3895)

* Returning 422 (instead 500) when query hit max_chunks_per_query limit

Signed-off-by: Alan Protasio <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Peter Štibraný <[email protected]>
Signed-off-by: Alan Protasio <[email protected]>

* Update CHANGELOG.md

Signed-off-by: Marco Pracucci <[email protected]>

Co-authored-by: Peter Štibraný <[email protected]>
Co-authored-by: Marco Pracucci <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -92,6 +92,7 @@
 * [BUGFIX] Compactor: fixed "could not guess file size" log when uploading blocks deletion marks to the global location. #3807
 * [BUGFIX] Prevent panic at start if the http_prefix setting doesn't have a valid value. #3796
 * [BUGFIX] Memberlist: fixed panic caused by race condition in `armon/go-metrics` used by memberlist client. #3724
+* [BUGFIX] Querier: returning 422 (instead of 500) when query hits `max_chunks_per_query` limit with block storage. #3895
 
 ## 1.7.0
 

pkg/querier/blocks_store_queryable.go

@@ -41,6 +41,7 @@ import (
 	"github.com/cortexproject/cortex/pkg/util/math"
 	"github.com/cortexproject/cortex/pkg/util/services"
 	"github.com/cortexproject/cortex/pkg/util/spanlogger"
+	"github.com/cortexproject/cortex/pkg/util/validation"
 )
 
 const (
@@ -614,7 +615,7 @@ func (q *blocksStoreQuerier) fetchSeriesFromStores(
 					if maxChunksLimit > 0 {
 						actual := numChunks.Add(int32(len(s.Chunks)))
 						if actual > int32(leftChunksLimit) {
-							return fmt.Errorf(errMaxChunksPerQueryLimit, convertMatchersToString(matchers), maxChunksLimit)
+							return validation.LimitError(fmt.Sprintf(errMaxChunksPerQueryLimit, convertMatchersToString(matchers), maxChunksLimit))
 						}
 					}
 				}

pkg/querier/blocks_store_queryable_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/cortexproject/cortex/pkg/storegateway/storegatewaypb"
 	"github.com/cortexproject/cortex/pkg/util"
 	"github.com/cortexproject/cortex/pkg/util/services"
+	"github.com/cortexproject/cortex/pkg/util/validation"
 )
 
 func TestBlocksStoreQuerier_Select(t *testing.T) {
@@ -67,18 +68,18 @@ func TestBlocksStoreQuerier_Select(t *testing.T) {
 		storeSetResponses []interface{}
 		limits            BlocksStoreLimits
 		expectedSeries    []seriesResult
-		expectedErr       string
+		expectedErr       error
 		expectedMetrics   string
 	}{
 		"no block in the storage matching the query time range": {
 			finderResult: nil,
 			limits:       &blocksStoreLimitsMock{},
-			expectedErr:  "",
+			expectedErr:  nil,
 		},
 		"error while finding blocks matching the query time range": {
 			finderErr:   errors.New("unable to find blocks"),
 			limits:      &blocksStoreLimitsMock{},
-			expectedErr: "unable to find blocks",
+			expectedErr: errors.New("unable to find blocks"),
 		},
 		"error while getting clients to query the store-gateway": {
 			finderResult: bucketindex.Blocks{
@@ -89,7 +90,7 @@ func TestBlocksStoreQuerier_Select(t *testing.T) {
 				errors.New("no client found"),
 			},
 			limits:      &blocksStoreLimitsMock{},
-			expectedErr: "no client found",
+			expectedErr: errors.New("no client found"),
 		},
 		"a single store-gateway instance holds the required blocks (single returned series)": {
 			finderResult: bucketindex.Blocks{
@@ -290,7 +291,7 @@ func TestBlocksStoreQuerier_Select(t *testing.T) {
 				errors.New("no store-gateway remaining after exclude"),
 			},
 			limits:      &blocksStoreLimitsMock{},
-			expectedErr: fmt.Sprintf("consistency check failed because some blocks were not queried: %s", block2.String()),
+			expectedErr: fmt.Errorf("consistency check failed because some blocks were not queried: %s", block2.String()),
 		},
 		"multiple store-gateway instances have some missing blocks (consistency check failed)": {
 			finderResult: bucketindex.Blocks{
@@ -315,7 +316,7 @@ func TestBlocksStoreQuerier_Select(t *testing.T) {
 				errors.New("no store-gateway remaining after exclude"),
 			},
 			limits:      &blocksStoreLimitsMock{},
-			expectedErr: fmt.Sprintf("consistency check failed because some blocks were not queried: %s %s", block3.String(), block4.String()),
+			expectedErr: fmt.Errorf("consistency check failed because some blocks were not queried: %s %s", block3.String(), block4.String()),
 		},
 		"multiple store-gateway instances have some missing blocks but queried from a replica during subsequent attempts": {
 			finderResult: bucketindex.Blocks{
@@ -435,7 +436,7 @@ func TestBlocksStoreQuerier_Select(t *testing.T) {
 				},
 			},
 			limits:      &blocksStoreLimitsMock{maxChunksPerQuery: 1},
-			expectedErr: fmt.Sprintf(errMaxChunksPerQueryLimit, fmt.Sprintf("{__name__=%q}", metricName), 1),
+			expectedErr: validation.LimitError(fmt.Sprintf(errMaxChunksPerQueryLimit, fmt.Sprintf("{__name__=%q}", metricName), 1)),
 		},
 		"max chunks per query limit hit while fetching chunks during subsequent attempts": {
 			finderResult: bucketindex.Blocks{
@@ -472,7 +473,7 @@ func TestBlocksStoreQuerier_Select(t *testing.T) {
 				},
 			},
 			limits:      &blocksStoreLimitsMock{maxChunksPerQuery: 3},
-			expectedErr: fmt.Sprintf(errMaxChunksPerQueryLimit, fmt.Sprintf("{__name__=%q}", metricName), 3),
+			expectedErr: validation.LimitError(fmt.Sprintf(errMaxChunksPerQueryLimit, fmt.Sprintf("{__name__=%q}", metricName), 3)),
 		},
 	}
 
@@ -502,8 +503,9 @@ func TestBlocksStoreQuerier_Select(t *testing.T) {
 			}
 
 			set := q.Select(true, nil, matchers...)
-			if testData.expectedErr != "" {
-				assert.EqualError(t, set.Err(), testData.expectedErr)
+			if testData.expectedErr != nil {
+				assert.EqualError(t, set.Err(), testData.expectedErr.Error())
+				assert.IsType(t, set.Err(), testData.expectedErr)
 				assert.False(t, set.Next())
 				assert.Nil(t, set.Warnings())
 				return