Clear all caches after testing parent breaker
With this commit we clear all caches after testing the parent circuit breaker. This is necessary as caches hold on to circuit breakers internally. Additionally, due to usage of CircuitBreaker#addWithoutBreaking() in caches, it's even possible to go above the limit. As a consequence, all subsequent requests fall victim to the limit. Hence, right after the parent circuit breaker tripped, we clear all caches to reduce these circuit breakers to 0 again. We also exclude the clear caches transport request from limit check in order to ensure it will succeed. As this is typically a very small and low-volume request, it is deemed ok to exclude it. Closes #18325
This commit is contained in:
parent
850e9d7c57
commit
c13df3b6c5
|
@ -54,7 +54,7 @@ public class TransportClearIndicesCacheAction extends TransportBroadcastByNodeAc
|
|||
TransportService transportService, IndicesService indicesService, ActionFilters actionFilters,
|
||||
IndexNameExpressionResolver indexNameExpressionResolver) {
|
||||
super(settings, ClearIndicesCacheAction.NAME, threadPool, clusterService, transportService, actionFilters, indexNameExpressionResolver,
|
||||
ClearIndicesCacheRequest::new, ThreadPool.Names.MANAGEMENT);
|
||||
ClearIndicesCacheRequest::new, ThreadPool.Names.MANAGEMENT, false);
|
||||
this.indicesService = indicesService;
|
||||
}
|
||||
|
||||
|
|
|
@ -84,6 +84,20 @@ public abstract class TransportBroadcastByNodeAction<Request extends BroadcastRe
|
|||
|
||||
final String transportNodeBroadcastAction;
|
||||
|
||||
public TransportBroadcastByNodeAction(
|
||||
Settings settings,
|
||||
String actionName,
|
||||
ThreadPool threadPool,
|
||||
ClusterService clusterService,
|
||||
TransportService transportService,
|
||||
ActionFilters actionFilters,
|
||||
IndexNameExpressionResolver indexNameExpressionResolver,
|
||||
Supplier<Request> request,
|
||||
String executor) {
|
||||
this(settings, actionName, threadPool, clusterService, transportService, actionFilters, indexNameExpressionResolver, request,
|
||||
executor, true);
|
||||
}
|
||||
|
||||
public TransportBroadcastByNodeAction(
|
||||
Settings settings,
|
||||
String actionName,
|
||||
|
@ -93,7 +107,8 @@ public abstract class TransportBroadcastByNodeAction<Request extends BroadcastRe
|
|||
ActionFilters actionFilters,
|
||||
IndexNameExpressionResolver indexNameExpressionResolver,
|
||||
Supplier<Request> request,
|
||||
String executor) {
|
||||
String executor,
|
||||
boolean canTripCircuitBreaker) {
|
||||
super(settings, actionName, threadPool, transportService, actionFilters, indexNameExpressionResolver, request);
|
||||
|
||||
this.clusterService = clusterService;
|
||||
|
@ -101,7 +116,8 @@ public abstract class TransportBroadcastByNodeAction<Request extends BroadcastRe
|
|||
|
||||
transportNodeBroadcastAction = actionName + "[n]";
|
||||
|
||||
transportService.registerRequestHandler(transportNodeBroadcastAction, NodeRequest::new, executor, new BroadcastByNodeTransportRequestHandler());
|
||||
transportService.registerRequestHandler(transportNodeBroadcastAction, NodeRequest::new, executor, false, canTripCircuitBreaker,
|
||||
new BroadcastByNodeTransportRequestHandler());
|
||||
}
|
||||
|
||||
private Response newResponse(
|
||||
|
|
|
@ -75,6 +75,9 @@ public class CircuitBreakerServiceIT extends ESIntegTestCase {
|
|||
/** Reset all breaker settings back to their defaults */
|
||||
private void reset() {
|
||||
logger.info("--> resetting breaker settings");
|
||||
// clear all caches, we could be very close (or even above) the limit and then we will not be able to reset the breaker settings
|
||||
client().admin().indices().prepareClearCache().setFieldDataCache(true).setQueryCache(true).setRequestCache(true).get();
|
||||
|
||||
Settings resetSettings = Settings.builder()
|
||||
.put(HierarchyCircuitBreakerService.FIELDDATA_CIRCUIT_BREAKER_LIMIT_SETTING.getKey(),
|
||||
HierarchyCircuitBreakerService.FIELDDATA_CIRCUIT_BREAKER_LIMIT_SETTING.getDefaultRaw(null))
|
||||
|
@ -214,7 +217,6 @@ public class CircuitBreakerServiceIT extends ESIntegTestCase {
|
|||
* Test that a breaker correctly redistributes to a different breaker, in
|
||||
* this case, the fielddata breaker borrows space from the request breaker
|
||||
*/
|
||||
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/18325")
|
||||
public void testParentChecking() throws Exception {
|
||||
if (noopBreakerUsed()) {
|
||||
logger.info("--> noop breakers used, skipping test");
|
||||
|
@ -274,9 +276,6 @@ public class CircuitBreakerServiceIT extends ESIntegTestCase {
|
|||
cause.toString(), startsWith("CircuitBreakingException[[parent] Data too large"));
|
||||
assertThat("Exception: [" + cause.toString() + "] should contain a CircuitBreakingException",
|
||||
cause.toString(), endsWith(errMsg));
|
||||
} finally {
|
||||
// reset before teardown as it requires properly set up breakers
|
||||
reset();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue