Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include hits in request circuit breaker #9310

Closed
clintongormley opened this issue Jan 15, 2015 · 7 comments
Closed

Include hits in request circuit breaker #9310

clintongormley opened this issue Jan 15, 2015 · 7 comments
Labels
:Core/Infra/Circuit Breakers Track estimates of memory consumption to prevent overload >enhancement high hanging fruit resiliency

Comments

@clintongormley
Copy link

The bytes from the _source field and highlighting returned during the FETCH phase should be included in the request circuit breaker.

@clintongormley clintongormley added v1.5.0 help wanted adoptme :Core/Infra/Circuit Breakers Track estimates of memory consumption to prevent overload >enhancement labels Jan 15, 2015
@jpountz
Copy link
Contributor

jpountz commented Jan 30, 2015

I think there are several things to think about:

  • do we want to do it on a per-request basis or globally (globally introduces serious complexity as we need to decrease the counter when memory is not used anymore, and use locks to ensure correctness of the counter)
  • we do not know how much memory stored fields use before we load them, so we could not really protect against super large documents
  • when serializing the response, we take the response object, serialize it, and only then start sending data over the network. So the actual total amount of memory that is used to handle the fetch phase would actually be close to twice the data that comes from stored fields + highlights.

Overall I am a bit concerned such a feature would be very hard to implement correctly while addressing #9311 would already solve most problems.

@kimchy
Copy link
Member

kimchy commented Jan 30, 2015

@jpountz I think this should use our request circuit breaker mechanism, that already works well in the context of aggs (in terms of releasing it at the correct search execution phase) and is "global". My thought was simply to add to the circuit breaker once each document is loaded. This will also help to take into account a more bigger notion of memory usage, basically anything the request circuit breaker encapsulates.

@clintongormley
Copy link
Author

@danielmitterdorfer does the inflight request circuit breaker handle this situation already?

@danielmitterdorfer
Copy link
Member

@clintongormley No, the inflight-request circuit breaker tracks only bytes for request processing (see TcpTransport#handleRequest()). Bytes allocated for responses are not counted.

@tomcallahan
Copy link
Contributor

@clintongormley can you elaborate on the priority of this item? This is on this page: https://www.elastic.co/guide/en/elasticsearch/resiliency/current/index.html

@Bukhtawar
Copy link
Contributor

Hey @jpountz when serializing the response, do you think it makes sense to have a hard limit on the response size, since we don't have any breakers for response neither do we have a limit on per doc size, only max docs/search buckets.

@danielmitterdorfer
Copy link
Member

Closing in favor of #42318.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Circuit Breakers Track estimates of memory consumption to prevent overload >enhancement high hanging fruit resiliency
Projects
None yet
Development

No branches or pull requests

8 participants