Cancel abandoned operations in ReplicationSet.Do() #3178
Conversation
Generally it will start a set of operations in parallel, and return once enough of them have succeeded. Pass down a context to each one, and cancel that context when ReplicationSet.Do() returns. Signed-off-by: Bryan Boreham <[email protected]>
Note the top-level context does get canceled when the request returns at top level, so the write path takes precautions to avoid being impacted by that: cortex/pkg/distributor/distributor.go Lines 535 to 536 in 3ef425d The reason why I wanted this PR in the read path is that requests in the querier can run for a while after ingester requests, e.g. evaluating PromQL over the data. |
| @@ -76,6 +76,7 @@ | |||
| * [BUGFIX] Configs: prevent validation of templates to fail when using template functions. #3157 | |||
| * [BUGFIX] Configuring the S3 URL with an `@` but without username and password doesn't enable the AWS static credentials anymore. #3170 | |||
| * [BUGFIX] Limit errors on ranged queries (`api/v1/query_range`) no longer return a status code `500` but `422` instead. #3167 | |||
| * [BUGFIX] No-longer-needed ingester operations from queries are now canceled. #3178 | |||
There was a problem hiding this comment.
I've just merged a PR to cut the CHANGELOG because of the upcoming 1.4.0 release. Could you rebase master and move your CHANGELOG entry to the top, under ## master / unreleased, please?
| @@ -76,6 +76,7 @@ | |||
| * [BUGFIX] Configs: prevent validation of templates to fail when using template functions. #3157 | |||
| * [BUGFIX] Configuring the S3 URL with an `@` but without username and password doesn't enable the AWS static credentials anymore. #3170 | |||
| * [BUGFIX] Limit errors on ranged queries (`api/v1/query_range`) no longer return a status code `500` but `422` instead. #3167 | |||
| * [BUGFIX] No-longer-needed ingester operations from queries are now canceled. #3178 | |||
There was a problem hiding this comment.
I left the same comment, but Bryan correctly told it's intended to be "queries" because it affects both queriers and rulers.
There was a problem hiding this comment.
Well, it confused two of us, so perhaps better wording like "operations caused by queries" would improve it?
Generally it will start a set of operations in parallel, and return once enough of them have succeeded. Pass down a context to each one, and cancel that context when ReplicationSet.Do() returns.
Fixes #3169
Checklist
CHANGELOG.mdupdated