Query-frontend worker: don't treat cancel as an error

[bboreham]

What this PR does

When a query returns an error value, check whether that error is actually a cancellation before acting.
I expect two effects: the worker won't log "context canceled", and it will process the next request immediately.

Could be the user closed the tab, or it's a sharded query and some other shard errored. Whatever the cause, cancellation is not an error.

Checklist

• NA Tests updated
• NA Documentation added
• CHANGELOG.md updated

[colega]

Package names, and aliases, should be all lowercase, so grpcutil.

Also an opinionated nit, but in this case I'd call it commongrpc.

[colega]

Can we write a test for this? These issues keep biting us again and again.

[bboreham]

Can we write a test for this? These issues keep biting us again and again.

I found it hard to think of a generic test to catch such issues by behaviour, but we could check all the logs from integration tests for the string "context canceled".

[colega]

LGTM, a test would be great, but I think we should prioritize delivering this. If we merge without a test, we could create a good-first-issue labeled issue to write a test.

[bboreham]

we could check all the logs from integration tests for the string "context canceled".

There are lots of reports when querier shuts down, e.g. here:

17:48:01 Stopping querier
17:48:01 querier: level=error ts=2023-04-03T17:48:01.927794752Z caller=frontend_processor.go:88 msg="error processing requests" address=172.26.0.6:9095 err="rpc error: code = Canceled desc = context canceled"
17:48:01 querier: level=error ts=2023-04-03T17:48:01.929005407Z caller=client.go:242 msg="error getting path" key=collectors/store-gateway err="Get \"http://e2e-mimir-test-consul:8500/v1/kv/collectors/store-gateway?index=24&stale=&wait=10000ms\": context canceled"
17:48:01 querier: level=error ts=2023-04-03T17:48:01.929028008Z caller=client.go:242 msg="error getting path" key=collectors/ring err="Get \"http://e2e-mimir-test-consul:8500/v1/kv/collectors/ring?index=18&stale=&wait=10000ms\": context canceled"

In TestQueryFrontendWithQueryShardingAndTooManyRequests we see dozens like this:

17:53:45 query-frontend: ts=2023-04-03T17:53:45.385880639Z caller=retry.go:78 level=error user=e2e-user traceID=3f3c6a4b164a9feb msg="error processing request" try=4 err="context canceled"
17:53:45 query-frontend: ts=2023-04-03T17:53:45.385983544Z caller=retry.go:78 level=error user=e2e-user traceID=3f3c6a4b164a9feb msg="error processing request" try=4 err="context canceled"
17:53:45 query-frontend: ts=2023-04-03T17:53:45.386079348Z caller=retry.go:78 level=error user=e2e-user traceID=3f3c6a4b164a9feb msg="error processing request" try=4 err="context canceled"

[lamida]

The CHANGELOG has just been cut to prepare for the next Mimir release. Please rebase main and eventually move the CHANGELOG entry added / updated in this PR to the top of the CHANGELOG document. Thanks!

[charleskorn]

I reviewed this post-merge and noticed that we likely need to make the same change for the case where the query-scheduler is in use here.

(frontendProcessor is only used by queriers when the query-scheduler is not in use.)

[bboreham]

Thanks for the pointer - filed #6767