Skip to content

Conversation

@iverase
Copy link
Contributor

@iverase iverase commented May 21, 2024

Currently when building global ordinals, we add to the breaker the size of the final object without breaking. I think the reason for not breaking is that it makes little sense to break at that point if we already build the object. The side effect is that we don't honour the breaker limit which is equally bad.

Therefore this commit proposes to add the bytes and break if we get over the limit of the breaker. the side effect is that working cluster might be hitting this breaker now. Still not honouring a limit should be considered a bug.

closes #97075

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 21, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine
Copy link
Collaborator

Hi @iverase, I've created a changelog YAML for you.

@iverase iverase requested a review from jpountz May 21, 2024 17:46
@nik9000
Copy link
Member

nik9000 commented May 21, 2024

I think this is the right thing to do, but:

  1. I'm sick. Don't trust me.
  2. I thought we already did this.
  3. I can see why we might not want to change it.

@benwtrent
Copy link
Member

This seems like the safe thing to do. Why wouldn't we want to check the CB on ordinal creation to protect users?

@iverase
Copy link
Contributor Author

iverase commented Jul 10, 2024

@elasticmachine update branch

@iverase
Copy link
Contributor Author

iverase commented Jul 10, 2024

@elasticmachine update branch

@wchaparro
Copy link
Member

@iverase Did we want to pursue this one?

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do it.

@dbeech
Copy link

dbeech commented Aug 21, 2025

@iverase @nik9000 is there any possibility to get this into 8.x ?

@iverase
Copy link
Contributor Author

iverase commented Nov 21, 2025

@elasticmachine please test this

@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.2
8.19
9.1

iverase added a commit to iverase/elasticsearch that referenced this pull request Nov 24, 2025
This commit adds the bytes and break if we get over the limit of the breaker when building global ordinals.
iverase added a commit to iverase/elasticsearch that referenced this pull request Nov 24, 2025
This commit adds the bytes and break if we get over the limit of the breaker when building global ordinals.
iverase added a commit to iverase/elasticsearch that referenced this pull request Nov 24, 2025
This commit adds the bytes and break if we get over the limit of the breaker when building global ordinals.
ywangd pushed a commit to ywangd/elasticsearch that referenced this pull request Nov 24, 2025
This commit adds the bytes and break if we get over the limit of the breaker when building global ordinals.
elasticsearchmachine pushed a commit that referenced this pull request Nov 24, 2025
This commit adds the bytes and break if we get over the limit of the breaker when building global ordinals.
elasticsearchmachine pushed a commit that referenced this pull request Nov 24, 2025
This commit adds the bytes and break if we get over the limit of the breaker when building global ordinals.
elasticsearchmachine pushed a commit that referenced this pull request Nov 24, 2025
This commit adds the bytes and break if we get over the limit of the breaker when building global ordinals.
ncordon pushed a commit to ncordon/elasticsearch that referenced this pull request Nov 26, 2025
This commit adds the bytes and break if we get over the limit of the breaker when building global ordinals.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations auto-backport Automatically create backport pull requests when merged >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.8 v9.1.8 v9.2.2 v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fielddata circuit breaker doesn't seem to limit cache size

8 participants