grafana / mimir-prometheus

Apache License 2.0
35 stars 9 forks source link

Improve context cancellation handling in PostingsForMatchersCache #644

Closed pracucci closed 4 months ago

pracucci commented 4 months ago

The PostingsForMatchersCache has a comment that says:

// FIXME: do we need to cancel the call to postingsForMatchers if all the callers waiting for the result have
// cancelled their context?

During the weekend we learned the hard way that, yes, we should. The reason is that the underlying PostingsForMatchers() honor context cancellation, but when it's called through the single flight promise used by the cache, we run it with a new context.Background() and so it never gets canceled.

If the execution of the underlying PostingsForMatchers() takes a very long time (e.g. due to an insanely complex regexp), then it's undesirable keep it running if the request has been canceled. We prefer to cancel the PostingsForMatchers() execution.

This PR addresses it.