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 name of the field that caused a circuit break in the log and exception message #5841

Merged
merged 1 commit into from Apr 23, 2014

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Apr 16, 2014

Fixes #5718

Now the log message looks like:

New used memory 19 [19b] from field [foo] would be larger than configured breaker: 15 [15b], breaking

And the exception on the client side looks like:

... ElasticsearchException[org.elasticsearch.common.breaker.CircuitBreakingException: Data too large, data for field [plot] would be larger than limit of [10485760/10mb]]; ...

@dakrone dakrone added v1.1.2 and removed v1.1.2 labels Apr 16, 2014
memoryBytesLimit + "/" + new ByteSizeValue(memoryBytesLimit) + "]");
}

public double addEstimateBytesAndMaybeBreak(long bytes) throws CircuitBreakingException {
Copy link
Member

Choose a reason for hiding this comment

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

Should this perhaps be marked @Deprecated or do you foresee a use for n/a fields (e.g., something else like whole documents?)?

If it's meant to allow something that is not a field, then maybe it should be a different message. If it is a field, then I would rather always see the field name.

Copy link
Member Author

Choose a reason for hiding this comment

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

I left it for testing simplicity, but it's simple enough to remove and force a field name, done.

@dakrone dakrone added the review label Apr 22, 2014
@s1monw
Copy link
Contributor

s1monw commented Apr 23, 2014

LGTM you should squash it and push

dakrone added a commit that referenced this pull request Apr 23, 2014
@dakrone dakrone merged commit b5adc87 into elastic:master Apr 23, 2014
mikemccand pushed a commit to mikemccand/elasticsearch that referenced this pull request Apr 24, 2014
@jpountz jpountz removed the review label Apr 30, 2014
@dakrone dakrone deleted the 5718-include-field-name-breaker branch September 9, 2014 13:48
@clintongormley clintongormley added the :Core/Infra/Circuit Breakers Track estimates of memory consumption to prevent overload label Jun 7, 2015
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 v1.2.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include name of field that caused circuit breaking exception
5 participants