Skip to content

Commit

Permalink
Salvacorts/max querier size messaging (#8916)
Browse files Browse the repository at this point in the history
**What this PR does / why we need it**:
In #8670 we introduced a new limit
`max_querier_bytes_read`. When the limit was surpassed the following
error message is printed:

```
query too large to execute on a single querier, either because parallelization is not enabled, the query is unshardable, or a shard query is too big to execute: (query: %s, limit: %s). Consider adding more specific stream selectors or reduce the time range of the query
```

As pointed out in [this comment][1], a user would have a hard time
figuring out whether the cause was `parallelization is not enabled`,
`the query is unshardable` or `a shard query is too big to execute`.

This PR improves the error messaging for the `max_querier_bytes_read`
limit to raise a different error for each of the causes above.

**Which issue(s) this PR fixes**:
Followup for #8670

**Special notes for your reviewer**:

**Checklist**
- [x] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [x] Documentation added
- [x] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/upgrading/_index.md`

[1]: #8670 (comment)

---------

Co-authored-by: Danny Kopping <[email protected]>
  • Loading branch information
salvacorts and Danny Kopping authored Mar 28, 2023
1 parent 44f1d8d commit 336e08f
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 13 deletions.
16 changes: 10 additions & 6 deletions pkg/querier/queryrange/limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ import (
)

const (
limitErrTmpl = "maximum of series (%d) reached for a single query"
maxSeriesErrTmpl = "max entries limit per query exceeded, limit > max_entries_limit (%d > %d)"
requiredLabelsErrTmpl = "stream selector is missing required matchers [%s], labels present in the query were [%s]"
limErrQueryTooManyBytesTmpl = "the query would read too many bytes (query: %s, limit: %s). Consider adding more specific stream selectors or reduce the time range of the query"
limErrQuerierTooManyBytesTmpl = "query too large to execute on a single querier, either because parallelization is not enabled, the query is unshardable, or a shard query is too big to execute: (query: %s, limit: %s). Consider adding more specific stream selectors or reduce the time range of the query"
limitErrTmpl = "maximum of series (%d) reached for a single query"
maxSeriesErrTmpl = "max entries limit per query exceeded, limit > max_entries_limit (%d > %d)"
requiredLabelsErrTmpl = "stream selector is missing required matchers [%s], labels present in the query were [%s]"
limErrQueryTooManyBytesTmpl = "the query would read too many bytes (query: %s, limit: %s); consider adding more specific stream selectors or reduce the time range of the query"
limErrQuerierTooManyBytesTmpl = "query too large to execute on a single querier: (query: %s, limit: %s); consider adding more specific stream selectors, reduce the time range of the query, or adjust parallelization settings"
limErrQuerierTooManyBytesUnshardableTmpl = "un-shardable query too large to execute on a single querier: (query: %s, limit: %s); consider adding more specific stream selectors or reduce the time range of the query"
limErrQuerierTooManyBytesShardableTmpl = "shard query is too large to execute on a single querier: (query: %s, limit: %s); consider adding more specific stream selectors or reduce the time range of the query"
)

var (
Expand Down Expand Up @@ -333,7 +335,9 @@ func (q *querySizeLimiter) guessLimitName() string {
if q.limitErrorTmpl == limErrQueryTooManyBytesTmpl {
return "MaxQueryBytesRead"
}
if q.limitErrorTmpl == limErrQuerierTooManyBytesTmpl {
if q.limitErrorTmpl == limErrQuerierTooManyBytesTmpl ||
q.limitErrorTmpl == limErrQuerierTooManyBytesShardableTmpl ||
q.limitErrorTmpl == limErrQuerierTooManyBytesUnshardableTmpl {
return "MaxQuerierBytesRead"
}
return "unknown"
Expand Down
12 changes: 9 additions & 3 deletions pkg/querier/queryrange/querysharding.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ type astMapperware struct {
maxShards int
}

func (ast *astMapperware) checkQuerySizeLimit(ctx context.Context, bytesPerShard uint64) error {
func (ast *astMapperware) checkQuerySizeLimit(ctx context.Context, bytesPerShard uint64, notShardable bool) error {
tenantIDs, err := tenant.TenantIDs(ctx)
if err != nil {
return httpgrpc.Errorf(http.StatusBadRequest, err.Error())
Expand All @@ -110,7 +110,13 @@ func (ast *astMapperware) checkQuerySizeLimit(ctx context.Context, bytesPerShard

if bytesPerShard > uint64(maxBytesRead) {
level.Warn(ast.logger).Log("msg", "Query exceeds limits", "status", "rejected", "limit_name", "MaxQuerierBytesRead", "limit_bytes", maxBytesReadStr, "resolved_bytes", statsBytesStr)
return httpgrpc.Errorf(http.StatusBadRequest, limErrQuerierTooManyBytesTmpl, statsBytesStr, maxBytesReadStr)

errorTmpl := limErrQuerierTooManyBytesShardableTmpl
if notShardable {
errorTmpl = limErrQuerierTooManyBytesUnshardableTmpl
}

return httpgrpc.Errorf(http.StatusBadRequest, errorTmpl, statsBytesStr, maxBytesReadStr)
}

level.Debug(ast.logger).Log("msg", "Query is within limits", "status", "accepted", "limit_name", "MaxQuerierBytesRead", "limit_bytes", maxBytesReadStr, "resolved_bytes", statsBytesStr)
Expand Down Expand Up @@ -168,7 +174,7 @@ func (ast *astMapperware) Do(ctx context.Context, r queryrangebase.Request) (que
level.Debug(logger).Log("no-op", noop, "mapped", parsed.String())

// Note, even if noop, bytesPerShard contains the bytes that'd be read for the whole expr without sharding
if err = ast.checkQuerySizeLimit(ctx, bytesPerShard); err != nil {
if err = ast.checkQuerySizeLimit(ctx, bytesPerShard, noop); err != nil {
return nil, err
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/querier/queryrange/querysharding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func Test_astMapper_QuerySizeLimits(t *testing.T) {
desc: "Non shardable query too big",
query: `sum_over_time({app="foo"} |= "foo" | unwrap foo [1h])`,
maxQuerierBytesSize: 10,
err: fmt.Sprintf(limErrQuerierTooManyBytesTmpl, "100 B", "10 B"),
err: fmt.Sprintf(limErrQuerierTooManyBytesUnshardableTmpl, "100 B", "10 B"),
expectedStatsHandlerHits: 1,
},
{
Expand All @@ -224,7 +224,7 @@ func Test_astMapper_QuerySizeLimits(t *testing.T) {
query: `count_over_time({app="foo"} |= "foo" [1h])`,
maxQuerierBytesSize: 10,

err: fmt.Sprintf(limErrQuerierTooManyBytesTmpl, "100 B", "10 B"),
err: fmt.Sprintf(limErrQuerierTooManyBytesShardableTmpl, "100 B", "10 B"),
expectedStatsHandlerHits: 1,
},
{
Expand All @@ -240,15 +240,15 @@ func Test_astMapper_QuerySizeLimits(t *testing.T) {
query: `count_over_time({app="bar"} |= "bar" [1h]) - sum_over_time({app="foo"} |= "foo" | unwrap foo [1h])`,
maxQuerierBytesSize: 100,

err: fmt.Sprintf(limErrQuerierTooManyBytesTmpl, "500 B", "100 B"),
err: fmt.Sprintf(limErrQuerierTooManyBytesShardableTmpl, "500 B", "100 B"),
expectedStatsHandlerHits: 2,
},
{
desc: "Partially Shardable RHS too big",
query: `count_over_time({app="foo"} |= "foo" [1h]) - sum_over_time({app="bar"} |= "bar" | unwrap foo [1h])`,
maxQuerierBytesSize: 100,

err: fmt.Sprintf(limErrQuerierTooManyBytesTmpl, "500 B", "100 B"),
err: fmt.Sprintf(limErrQuerierTooManyBytesShardableTmpl, "500 B", "100 B"),
expectedStatsHandlerHits: 2,
},
} {
Expand Down

0 comments on commit 336e08f

Please sign in to comment.