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

Add circuit breaker for parent/child id cache #5325

Closed

Conversation

Projects
None yet
4 participants
@dakrone
Copy link
Member

dakrone commented Mar 3, 2014

Changes include:

  • Bumping the flush size for RamAccountingTermsEnum to reduce log messages when TRACE logging is enabled
  • A breaker for parent/child id cache that I've tested against 20 million docs (10m parents, 10m children, 1 parent per child, parents having incremental ids & children having randomly generated ids) as well as the stackoverflow data (see graph, x axis is the segment #, y axis is kb adjustment after segment is loaded)
  • Fix for a bug I discovered that made the circuit breaker revert to 80% of the heap size even if it was configured in elasticsearch.yml

adj-double-term-length

Closes #5325

@dakrone dakrone added v1.1.0 labels Mar 3, 2014

@dakrone

This comment has been minimized.

Copy link
Member Author

dakrone commented Mar 3, 2014

Also for reference, the graph is for ~168mb of parent/child id field data, so an adjustment of 3.75mb is pretty accurate (because we want the estimation err on the high side rather than the low side).

@spinscale

View changes

src/main/java/org/elasticsearch/indices/fielddata/breaker/InternalCircuitBreakerService.java Outdated
@@ -59,7 +59,7 @@ public InternalCircuitBreakerService(Settings settings, NodeSettingsService node
@Override
public void onRefreshSettings(Settings settings) {
// clear breaker now that settings have changed
long newMaxByteSizeValue = settings.getAsMemory(CIRCUIT_BREAKER_MAX_BYTES_SETTING, DEFAULT_BREAKER_LIMIT).bytes();
long newMaxByteSizeValue = settings.getAsMemory(CIRCUIT_BREAKER_MAX_BYTES_SETTING, maxBytes + "b").bytes();

This comment has been minimized.

Copy link
@spinscale

spinscale Mar 4, 2014

Member

cant you just store maxBytes as a byte size value to prevent that string concatenation?

@martijnvg

This comment has been minimized.

Copy link
Member

martijnvg commented Mar 4, 2014

The changes regarding the circuit breaker to p/c look good! Perhaps the other changes should have their own PR / issue? (maybe we want to port the other commits to 1.0?)

@dakrone

This comment has been minimized.

Copy link
Member Author

dakrone commented Mar 4, 2014

That's a good idea, I'll split this out and issue 3 different PRs

@dakrone dakrone removed the bug label Mar 4, 2014

@martijnvg

This comment has been minimized.

Copy link
Member

martijnvg commented Mar 4, 2014

+1 This looks good to me!

@dakrone

This comment has been minimized.

Copy link
Member Author

dakrone commented Mar 4, 2014

Pushed to master and 1.x in 23471cd and d15d2b4

@dakrone dakrone closed this Mar 4, 2014

@dakrone dakrone deleted the dakrone:5276-parent-child-circuit-break branch Apr 21, 2014

@clintongormley clintongormley changed the title add circuit breaker for parent/child id cache Add circuit breaker for parent/child id cache Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.