Skip to content

Disable Real Memory Circuit Breaker by default and deprecate it#112115

Closed
original-brownbear wants to merge 3 commits intoelastic:mainfrom
original-brownbear:flip-default-rmcb
Closed

Disable Real Memory Circuit Breaker by default and deprecate it#112115
original-brownbear wants to merge 3 commits intoelastic:mainfrom
original-brownbear:flip-default-rmcb

Conversation

@original-brownbear
Copy link
Contributor

As discussed, deprecating the setting and flipping it to false by default on account of the progress we made that makes this breaker hurt more than it helps.

As discussed, deprecating the setting and flipping it to `false` by
default on account of the progress we made that makes this breaker hurt
more than it helps.
@original-brownbear original-brownbear added >non-issue :Core/Infra/Core Core issues without another label labels Aug 22, 2024
@github-actions
Copy link
Contributor

Documentation preview:

@elasticsearchmachine elasticsearchmachine added v8.16.0 Team:Core/Infra Meta label for core/infra team labels Aug 22, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@original-brownbear original-brownbear requested a review from a team as a code owner August 28, 2024 12:41
@iverase
Copy link
Contributor

iverase commented Aug 28, 2024

I understand the motivation of this change and I agree with it. Still I am concerned that some parts of the code rely on the real memory circuit breaker in order to avoid OOMs. This change will leave those cases totally out of coverage so I think we need to provide some other mechanism in order to have some protection against OOMs.

The cases I am aware of:

breaker.addEstimateBytesAndMaybeBreak(0, "allocated_buckets");

This is pretty important and the reason we increase the max number of buckets from 10k to 65k as it will give the protection in case we will be crashing too much data on heap on the coordinator node during reduction.

breaker.addEstimateBytesAndMaybeBreak(0, "allocated_buckets");

Similar to the above but in the data node. There are some objects in the aggregation framework that are not memory tracked.

breaker.addEstimateBytesAndMaybeBreak(0L, "Global Ordinals");

That was added recently when we notice that building global ordinals can hit OOMs in clusters with small heap.

That was added recently when we notice that building the filters on a filters aggregator can hit OOMs in clusters with small heap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v9.3.0

4 participants