Add optional querier response streaming

[flxbk]

What this PR does

Some Mimir API endpoints can generate responses with a very large response body. Today, responses are transmitted from queriers to query-frontends via the non-streaming gRPC call

rpc QueryResult (QueryResultRequest) returns (QueryResultResponse) { };

In case of the /series and /active_series APIs, the (partial) responses generated by queriers can get so large that they cause the query-frontend handling the request to OOM on unmarshaling the QueryResultRequest containing the response. The following example profile from an internal cell demonstrates this behaviour:

While queriers need to accumulate the entire response in memory to be able to deduplicate results from ingesters, query-frontends do not need to bring the full response into memory in all cases. The "active series" endpoint in particular never needs to do this and was implemented in a streaming fashion to avoid the issue of OOMs for large result sets. However, since all response bodies for all types of queries are fully loaded into query-frontend memory in today's implementation of the communication between queriers and query-frontends, this streaming is essentially useless.

To avoid this issue and enable a single query-frontend instance to process arbitrarily large responses, this PR proposes the introduction of a new (streaming) rpc

rpc QueryResultStream (stream QueryResultStreamRequest) returns (QueryResultResponse) { };

that queriers can invoke instead of the QueryResult call to transmit responses to query-frontends in a streaming fashion. This essentially requires two changes:

• Lifecycle management of requests in the query-frontend needs to be handled differently. This is because a request's context and its associated frontendRequest data-structure can no longer be canceled/cleaned up on return from RoundTripGRPC. Instead, the required cleanup is captured in a closure and passed to the caller, who must invoke it through closing the response body once the transfer is complete. Because of that, we risk introducing a memory leak that could be triggered by client code not closing the response body (today not closing the response body would not lead to a leak, because the body is an io.NopCloser).
• Queriers need to use the new rpc to stream responses. This PR puts that behaviour behind a feature flag and also requires the response being sent to indicate it wants to use streaming via a header. In this PR, only responses to the /active_series endpoint request the streaming behaviour, all other responses will continue to be transmitted via the existing rpc even when the feature flag is enabled.

One less-than-obvious caveat of this streaming behaviour is that queriers participate in the handling of the request for longer than they previously would have (namely until the response body is fully streamed into the query-frontend, which itself may block on streaming to the client). Since request concurrency in queriers is limited by the -querier.max-concurrent flag, this may lead to slow clients and/or big requests blocking querier workers. The following pair of example traces with and without streaming illustrates this difference.

Without response streaming

With response streaming

If this approach proves viable, other request types might also benefit from streaming in the future (e.g. remote read).

Which issue(s) this PR fixes or relates to

n/a

Checklist

• Tests updated.
• Documentation added.
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
• about-versioning.md updated with experimental features.

[dimitarvdimitrov]

can you add a few sentences to the description with some more details for how this works? it will make the review easier. Or if you have a design doc, that'd suffice too

[flxbk]

Thanks for the suggestion and excuse the previously barren PR description @dimitarvdimitrov, I have added some context.

[dimitarvdimitrov]

I had a high-level look and the implementation looks good. It fits fairly naturally in the current model. I will take a deeper look later on too.

Perhaps if we want to take this one step further I would have done the preliminary work wrt adding a streaming proto file in httpgrpc instead of only in frontend.proto. I am less hopeful that we will actually do proper streaming httpgrpc because it will require changing a lot of code, but decided to bring it up.

Would be nice to sync with @aknuds1 on his plans for streaming to make sure we don't implement the same thing in two different ways.

[flxbk]

Perhaps if we want to take this one step further I would have done the preliminary work wrt adding a streaming proto file in httpgrpc instead of only in frontend.proto. I am less hopeful that we will actually do proper streaming httpgrpc because it will require changing a lot of code, but decided to bring it up.

From internal discussions I gathered that the general sentiment in the team is that streaming through the frontend is not very useful for most endpoints (e.g. because of caching done by frontends). Given that, I decided it would be better to keep the change as local as possible for now.

[dimitarvdimitrov]

very solid work! 💪

My main comments are around making the tests a bit more elaborate, most of the others are nitpicks, otherwise I couldn't find anything surprising

[dimitarvdimitrov]

final set of comments and then we can merge :)