Improve DURABILITY logic for circuit breaker, make it actionable

[Bukhtawar]

Problem Description

While real memory circuit breaker helps with parent memory accounting based on real heap usage the DURABILITY is still derived from child circuit breaker, the logic for which is based on whichever contributed the maximum based on their respective durability. Now child circuit breakers are known to be not so accurate, as a result, it gets harder to derive the nature of issue resulting in frequent GC or request trips.

Keeping a node around which has request tripping frequently isn't optimal and decays into the throughput, in some cases it may be deemed better to bounce off such nodes to clear garbage.

Sharing a sample instance showing how real memory(parent) and child could be totally off

curl localhost:9200/_cat/thread_pool?v

{"error":{"root_cause":[{"type":"circuit_breaking_exception","reason":"[parent] Data too large, data for [<http_request>] would be [8337839048/7.7gb], which is larger than the limit of [8127315968/7.5gb], real usage: [8337839048/7.7gb], new bytes reserved: [0/0b], usages [request=0/0b, fielddata=0/0b, in_flight_requests=0/0b, accounting=6388018/6mb]","bytes_wanted":8337839048,"bytes_limit":8127315968,"durability":"PERMANENT"}],"type":"circuit_breaking_exception","reason":"[parent] Data too large, data for [<http_request>] would be [8337839048/7.7gb], which is larger than the limit of [8127315968/7.5gb], real usage: [8337839048/7.7gb], new bytes reserved: [0/0b], usages [request=0/0b, fielddata=0/0b, in_flight_requests=0/0b, accounting=6388018/6mb]","bytes_wanted":8337839048,"bytes_limit":8127315968 .....

Proposal

Derive durability based on some function of

1. Request trips over a period of time(avoiding flip flops)
2. Heap after GC(ensuring GC throughput is still reasonable)

Should nodes still be considered healthy(as a part of the cluster) if nodes continue to trip majority of the requests over a prolonged period of time or even with the present PERMANENT durability nature of circuit breaker. Thoughts?

[elasticmachine]

Pinging @elastic/es-core-infra (:Core/Infra/Circuit Breakers)

[Bukhtawar]

The other thing I want to bring up is if the nature is PERMANENT the desired action to remediate is not clear(or even documented).
So lets say field data/accounting circuit breaker is the reason for PERMANENT durability. Should a shard allocator for instance cause the shard to move-off the node to alleviate memory pressure.

[dakrone]

I think it could make sense to make the parent breaker (or any of the breakers, actually) have a durability that can change.

For instance, along the lines of your first idea, I think we could implement a token bucket for the number of times that the breaker trips within an interval. We could set it to something sane where exhausting the bucket (ie, tripping X times within N seconds/minutes) causes the durability of the circuit breaking exception to be switched from TRANSIENT to PERMANENT.

The implementation for this would be similar to the script compilation circuit breaker, which already uses a token bucket for limiting the amount of script complications within a certain amount of time.

[Bukhtawar]

Thanks @dakrone if the request rate is low and the percentage of trip is high, lets say with the limit of 100/min, lets say the node receives just 50 and trips for all, should that case not be PERMANENT too. Wondering if percentage of request trip adds value.

Do you think a PERMANENT durability should be actionable as well

[dakrone]

Do you think a PERMANENT durability should be actionable as well

I think that depends on what we consider actionable, the permanent durability label is intended to signify that the error is unlikely to go away without some kind of human user intervention; so I guess I am asking what you mean by "actionable"?

[Bukhtawar]

Possible actions

1. Moving the shards(eg: top field data consumer) off the node
2. Clearing cache
3. Trip ping requests too, so that the node is removed from the cluster

Edit: I'll be interested on working on the proposal to fix the durability first though

[dakrone]

Hmm... those options I am concerned with. For any of those to be effective requires a holistic view of more than the single node in question—moving a shard for example—may exacerbate problems that other nodes may also be experiencing, clearing the cache may lead to cache churn (assuming that the data in the cache is being used) and hide the underlying problem of a cache that needs to have its limits adjusted, and removing the node from the cluster would redirect that load to the other nodes, potentially bringing them down.

The purpose of the circuit breakers is to prevent a subset of actions (indexing, searching), in order to allow the local node to continue to exist. In these cases we either break our intended purpose (removing the node from the cluster would essentially be reverting to letting it go out of memory and die), or we would shift the burden to other nodes without fully understanding the consequences.

The behavior you're describing is resource management operating at a level above the current circuit breaker architecture (cluster level versus node level), and while it would be interesting and maybe worthwhile to pursue, I think it's out of scope for the actual circuit breaker and circuit breaker service to handle.

[dakrone]

I'll be interested on working on the proposal to fix the durability first though

I do think this is worth pursuing, to be clear, I am just not convinced for the other possible actions you mentioned

[Bukhtawar]

Thanks @dakrone I agree with you as long as we can reliably mark a node to be broken(reflective of a permanent issue requiring manual intervention), it should still be fine. Let me know if percentage based trip approach within a moving window works. I'll get started on a PR to address the same