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

Circuit-break on response size #42318

Closed
jimczi opened this issue May 21, 2019 · 9 comments
Closed

Circuit-break on response size #42318

jimczi opened this issue May 21, 2019 · 9 comments
Labels
:Core/Infra/Circuit Breakers Track estimates of memory consumption to prevent overload >feature resiliency Team:Core/Infra Meta label for core/infra team

Comments

@jimczi
Copy link
Contributor

jimczi commented May 21, 2019

We have a circuit breaker for in_flight request but we don't check the response size even for APIs that can return big json output. For instance getting all _segments in a big cluster with lot of indices can kill the node responsible for the request since it needs to gather a lot of internal response as well as the final response that needs to be serialized in json. It could be beneficial to add a circuit breaker that checks the size of the transport responses when executing internal requests through the transport layer and to also control the memory usage of the response that is serialized at the rest layer. The latter is more important due to the json serialization that can be very verbose.

@jimczi jimczi added >feature :Core/Infra/Circuit Breakers Track estimates of memory consumption to prevent overload labels May 21, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jimczi
Copy link
Contributor Author

jimczi commented May 21, 2019

\cc @ywelsch @DaveCTurner

@jaymode
Copy link
Member

jaymode commented Jun 3, 2019

The es-core-infra team discussed and we believe that this is something worth implementing as a way to add additional safeguards.

@danielmitterdorfer
Copy link
Member

IMHO this is related to #9310. As this issue is formulated in a more generic fashion I'll close #9310 in favor of this one.

@felix-lessoer
Copy link

felix-lessoer commented Aug 11, 2020

@elastic/es-core-infra Any updates or workarounds available?

@jasontedor
Copy link
Member

@felix-lessoer This is not actively being worked on at this time. It’s on our list, but we are addressing more important priorities currently.

@rjernst rjernst added the needs:triage Requires assignment of a team area label label Dec 3, 2020
@rjernst rjernst removed the needs:triage Requires assignment of a team area label label Dec 15, 2020
@Leaf-Lin
Copy link
Contributor

Leaf-Lin commented Dec 3, 2021

I added the resiliency label because this was mentioned in the original issue #9310 and I still see this issue surfacing today.

The above issue was also mentioned from the https://www.elastic.co/guide/en/elasticsearch/resiliency/current/index.html page where we stated the STATUS is still ongoing:

Add the byte size of each hit to the request circuit breaker #9310. (STATUS: ONGOING)

@vineelyalamarthy
Copy link

Are contributors welcome to look into this?

@original-brownbear
Copy link
Member

This has become unnecessary with the introduction of chunked rest responses which turn the amount of memory needed for serialising the mentioned responses O(1). Once we're through converting all APIs that might result in a large response to chunked encoding there's no need for a circuit breaker.
relates #89838 -> closing here

@original-brownbear original-brownbear closed this as not planned Won't fix, can't repro, duplicate, stale Nov 30, 2022
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 >feature resiliency Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

No branches or pull requests

10 participants