sigram commented on a change in pull request #1626: URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447586354
########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.solr.core.SolrConfig; + +/** + * Manages all registered circuit breaker instances. Responsible for a holistic view + * of whether a circuit breaker has tripped or not. + * + * There are two typical ways of using this class's instance: + * 1. Check if any circuit breaker has triggered -- and know which circuit breaker has triggered. + * 2. Get an instance of a specific circuit breaker and perform checks. + * + * It is a good practice to register new circuit breakers here if you want them checked for every + * request. + * + * NOTE: The current way of registering new default circuit breakers is minimal and not a long term + * solution. There will be a follow up with a SIP for a schema API design. + */ +public class CircuitBreakerManager { + + private final Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>(); + + // Allows replacing of existing circuit breaker + public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) { + circuitBreakerMap.put(circuitBreakerType, circuitBreaker); + } + + public CircuitBreaker getCircuitBreaker(CircuitBreakerType circuitBreakerType) { + assert circuitBreakerType != null; + + return circuitBreakerMap.get(circuitBreakerType); + } + + /** + * Check if any circuit breaker has triggered. + * @return CircuitBreakers which have triggered, null otherwise + */ + public Map<CircuitBreakerType, CircuitBreaker> checkedTrippedCircuitBreakers() { + Map<CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null; + + for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) { + CircuitBreaker circuitBreaker = entry.getValue(); + + if (circuitBreaker.isEnabled() && + circuitBreaker.isTripped()) { + if (triggeredCircuitBreakers == null) { + triggeredCircuitBreakers = new HashMap<>(); + } + + triggeredCircuitBreakers.put(entry.getKey(), circuitBreaker); + } + } + + return triggeredCircuitBreakers; + } + + /** + * Returns true if *any* circuit breaker has triggered, false if none have triggered + * + * NOTE: This method short circuits the checking of circuit breakers -- the method will + * return as soon as it finds a circuit breaker that is enabled and has triggered + */ + public boolean checkAnyCircuitBreakerTripped() { Review comment: `checkAnyTripped` ? ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.solr.core.SolrCore; + +/** + * Manages all registered circuit breaker instances. Responsible for a holistic view + * of whether a circuit breaker has tripped or not. + * + * There are two typical ways of using this class's instance: + * 1. Check if any circuit breaker has triggered -- and know which circuit breaker has triggered. + * 2. Get an instance of a specific circuit breaker and perform checks. + * + * It is a good practice to register new circuit breakers here if you want them checked for every + * request. + * + * NOTE: The current way of registering new default circuit breakers is minimal and not a long term + * solution. There will be a follow up with a SIP for a schema API design. + */ +public class CircuitBreakerManager { + + private final Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>(); + + // Allows replacing of existing circuit breaker + public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) { + circuitBreakerMap.put(circuitBreakerType, circuitBreaker); + } + + public CircuitBreaker getCircuitBreaker(CircuitBreakerType circuitBreakerType) { + assert circuitBreakerType != null; + + return circuitBreakerMap.get(circuitBreakerType); + } + + /** + * Check if any circuit breaker has triggered. + * @return CircuitBreakers which have triggered, null otherwise + */ + public Map<CircuitBreakerType, CircuitBreaker> checkAllCircuitBreakersAndReturnTrippedBreakers() { + Map<CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null; Review comment: There's a typo in this method name - "checked" -> "check". In general, could we use shorter method names rather than these very long ones? We already know what the component does, we know that it manages CircuitBreakers - IMHO there's no need whatsoever to remind users in every method name that they deal with CircuitBreakers. :) I propose `getTripped`, `checkAnyTripped`, `toErrorMessage` and so on, avoiding repetition where it's obvious what we're dealing with. ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerType.java ########## @@ -0,0 +1,26 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +/** + * Types of circuit breakers + */ Review comment: I think Erick suggested (and I agree) that this enum should be simply declared in `CircuitBreaker` class, as `CircuitBreaker.CircuitBreakerType` or just `CircuitBreaker.Type`. This doesn't affect compilation but helps to reduce the number of trivial class files. This is also a pattern that is used frequently in Solr in other places. ########## File path: solr/solr-ref-guide/src/circuit-breakers.adoc ########## @@ -0,0 +1,65 @@ += Circuit Breakers +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +Solr's circuit breaker infrastructure allows prevention of actions that can cause a node to go beyond its capacity or to go down. The +premise of circuit breakers is to ensure a higher quality of service and only accept request loads that are serviceable in the current +resource configuration. + +== When To Use Circuit Breakers +Circuit breakers should be used when the user wishes to trade request throughput for a higher Solr stability. If circuit breakers +are enabled, requests may be rejected under the condition of high node duress with an appropriate HTTP error code (typically 503). + +It is upto the client to handle the same and potentially build a retrial logic as this should ideally be a transient situation. + +== Circuit Breaker Configurations +The following flag controls the global activation/deactivation of circuit breakers. If this flag is disabled, all circuit breakers +will be disabled globally. Per circuit breaker configurations are specified in their respective sections later. + +[source,xml] +---- +<useCircuitBreakers>false</useCircuitBreakers> +---- + +== Currently Supported Circuit Breakers + +=== JVM Heap Usage Based Circuit Breaker +This circuit breaker tracks JVM heap memory usage and rejects incoming search requests with a 503 error code if the heap usage +exceeds a configured percentage of maximum heap allocated to the JVM (-XMx). The main configuration for this circuit breaker is Review comment: `-Xmx` ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java ########## @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.lang.management.ManagementFactory; +import java.lang.management.MemoryMXBean; + +import org.apache.solr.core.SolrConfig; + +/** + * <p> + * Tracks the current JVM heap usage and triggers if it exceeds the defined percentage of the maximum + * heap size allocated to the JVM. This circuit breaker is a part of the default CircuitBreakerManager + * so is checked for every request -- hence it is realtime. Once the memory usage goes below the threshold, + * it will start allowing queries again. + * </p> + * + * <p> + * The memory threshold is defined as a percentage of the maximum memory allocated -- see memoryCircuitBreakerThreshold + * in solrconfig.xml + * </p> + */ + +public class MemoryCircuitBreaker extends CircuitBreaker { + private static final MemoryMXBean MEMORY_MX_BEAN = ManagementFactory.getMemoryMXBean(); + + private final long heapMemoryThreshold; + + // Assumption -- the value of these parameters will be set correctly before invoking printDebugInfo() + private ThreadLocal<Long> seenMemory = new ThreadLocal<>(); Review comment: These can be final too (it doesn't matter at runtime but it makes the intent clear). ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.solr.core.SolrConfig; + +/** + * Manages all registered circuit breaker instances. Responsible for a holistic view + * of whether a circuit breaker has tripped or not. + * + * There are two typical ways of using this class's instance: + * 1. Check if any circuit breaker has triggered -- and know which circuit breaker has triggered. + * 2. Get an instance of a specific circuit breaker and perform checks. + * + * It is a good practice to register new circuit breakers here if you want them checked for every + * request. + * + * NOTE: The current way of registering new default circuit breakers is minimal and not a long term + * solution. There will be a follow up with a SIP for a schema API design. + */ +public class CircuitBreakerManager { + + private final Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>(); + + // Allows replacing of existing circuit breaker + public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) { + circuitBreakerMap.put(circuitBreakerType, circuitBreaker); + } + + public CircuitBreaker getCircuitBreaker(CircuitBreakerType circuitBreakerType) { + assert circuitBreakerType != null; + + return circuitBreakerMap.get(circuitBreakerType); + } + + /** + * Check if any circuit breaker has triggered. + * @return CircuitBreakers which have triggered, null otherwise + */ + public Map<CircuitBreakerType, CircuitBreaker> checkedTrippedCircuitBreakers() { + Map<CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null; + + for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) { + CircuitBreaker circuitBreaker = entry.getValue(); + + if (circuitBreaker.isEnabled() && + circuitBreaker.isTripped()) { + if (triggeredCircuitBreakers == null) { + triggeredCircuitBreakers = new HashMap<>(); + } + + triggeredCircuitBreakers.put(entry.getKey(), circuitBreaker); + } + } + + return triggeredCircuitBreakers; + } + + /** + * Returns true if *any* circuit breaker has triggered, false if none have triggered + * + * NOTE: This method short circuits the checking of circuit breakers -- the method will + * return as soon as it finds a circuit breaker that is enabled and has triggered + */ + public boolean checkAnyCircuitBreakerTripped() { + for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) { + CircuitBreaker circuitBreaker = entry.getValue(); + + if (circuitBreaker.isEnabled() && + circuitBreaker.isTripped()) { + return true; + } + } + + return false; + } + + /** + * Construct the final error message to be printed when circuit breakers trip + * @param circuitBreakerMap Input list for circuit breakers + * @return Constructed error message + */ + public static String constructFinalErrorMessageString(Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap) { Review comment: This is an awkward name - maybe `toErrorString` or `toErrorMessage` ? ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.solr.core.SolrConfig; + +/** + * Manages all registered circuit breaker instances. Responsible for a holistic view + * of whether a circuit breaker has tripped or not. + * + * There are two typical ways of using this class's instance: + * 1. Check if any circuit breaker has triggered -- and know which circuit breaker has triggered. + * 2. Get an instance of a specific circuit breaker and perform checks. + * + * It is a good practice to register new circuit breakers here if you want them checked for every + * request. + * + * NOTE: The current way of registering new default circuit breakers is minimal and not a long term + * solution. There will be a follow up with a SIP for a schema API design. + */ +public class CircuitBreakerManager { + + private final Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>(); + + // Allows replacing of existing circuit breaker + public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) { Review comment: We can drop CircuitBreaker from the method name - we already know what we're going to register. ########## File path: solr/solr-ref-guide/src/circuit-breakers.adoc ########## @@ -0,0 +1,65 @@ += Circuit Breakers +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +Solr's circuit breaker infrastructure allows prevention of actions that can cause a node to go beyond its capacity or to go down. The +premise of circuit breakers is to ensure a higher quality of service and only accept request loads that are serviceable in the current +resource configuration. + +== When To Use Circuit Breakers +Circuit breakers should be used when the user wishes to trade request throughput for a higher Solr stability. If circuit breakers +are enabled, requests may be rejected under the condition of high node duress with an appropriate HTTP error code (typically 503). + +It is upto the client to handle the same and potentially build a retrial logic as this should ideally be a transient situation. Review comment: upto -> up to handle the same -> handle this error ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.solr.core.SolrConfig; + +/** + * Manages all registered circuit breaker instances. Responsible for a holistic view + * of whether a circuit breaker has tripped or not. + * + * There are two typical ways of using this class's instance: + * 1. Check if any circuit breaker has triggered -- and know which circuit breaker has triggered. + * 2. Get an instance of a specific circuit breaker and perform checks. + * + * It is a good practice to register new circuit breakers here if you want them checked for every + * request. + * + * NOTE: The current way of registering new default circuit breakers is minimal and not a long term + * solution. There will be a follow up with a SIP for a schema API design. + */ +public class CircuitBreakerManager { + + private final Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>(); + + // Allows replacing of existing circuit breaker + public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) { + circuitBreakerMap.put(circuitBreakerType, circuitBreaker); + } + + public CircuitBreaker getCircuitBreaker(CircuitBreakerType circuitBreakerType) { + assert circuitBreakerType != null; + + return circuitBreakerMap.get(circuitBreakerType); + } + + /** + * Check if any circuit breaker has triggered. + * @return CircuitBreakers which have triggered, null otherwise + */ + public Map<CircuitBreakerType, CircuitBreaker> checkedTrippedCircuitBreakers() { Review comment: Drop CircuitBreakers? ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.solr.core.SolrConfig; + +/** + * Manages all registered circuit breaker instances. Responsible for a holistic view + * of whether a circuit breaker has tripped or not. + * + * There are two typical ways of using this class's instance: + * 1. Check if any circuit breaker has triggered -- and know which circuit breaker has triggered. + * 2. Get an instance of a specific circuit breaker and perform checks. + * + * It is a good practice to register new circuit breakers here if you want them checked for every + * request. + * + * NOTE: The current way of registering new default circuit breakers is minimal and not a long term + * solution. There will be a follow up with a SIP for a schema API design. + */ +public class CircuitBreakerManager { + + private final Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>(); + + // Allows replacing of existing circuit breaker + public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) { + circuitBreakerMap.put(circuitBreakerType, circuitBreaker); + } + + public CircuitBreaker getCircuitBreaker(CircuitBreakerType circuitBreakerType) { + assert circuitBreakerType != null; + + return circuitBreakerMap.get(circuitBreakerType); + } + + /** + * Check if any circuit breaker has triggered. + * @return CircuitBreakers which have triggered, null otherwise + */ + public Map<CircuitBreakerType, CircuitBreaker> checkedTrippedCircuitBreakers() { + Map<CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null; + + for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) { + CircuitBreaker circuitBreaker = entry.getValue(); + + if (circuitBreaker.isEnabled() && + circuitBreaker.isTripped()) { + if (triggeredCircuitBreakers == null) { + triggeredCircuitBreakers = new HashMap<>(); + } + + triggeredCircuitBreakers.put(entry.getKey(), circuitBreaker); + } + } + + return triggeredCircuitBreakers; + } + + /** + * Returns true if *any* circuit breaker has triggered, false if none have triggered + * + * NOTE: This method short circuits the checking of circuit breakers -- the method will + * return as soon as it finds a circuit breaker that is enabled and has triggered + */ + public boolean checkAnyCircuitBreakerTripped() { + for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) { + CircuitBreaker circuitBreaker = entry.getValue(); + + if (circuitBreaker.isEnabled() && + circuitBreaker.isTripped()) { + return true; + } + } + + return false; + } + + /** + * Construct the final error message to be printed when circuit breakers trip + * @param circuitBreakerMap Input list for circuit breakers + * @return Constructed error message + */ + public static String constructFinalErrorMessageString(Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap) { + assert circuitBreakerMap != null; + + StringBuilder sb = new StringBuilder(); + + for (CircuitBreakerType circuitBreakerType : circuitBreakerMap.keySet()) { + sb.append(circuitBreakerType.toString() + " " + circuitBreakerMap.get(circuitBreakerType).getDebugInfo()); + } + + return sb.toString(); + } + + /** + * Register default circuit breakers and return a constructed CircuitBreakerManager + * instance which serves the given circuit breakers. + * + * Any default circuit breakers should be registered here + */ + public static CircuitBreakerManager buildDefaultCircuitBreakerManager(SolrConfig solrConfig) { Review comment: If circuit breakers are not enabled in SolrConfig is there still any point to register any of them? Also, I propose to rename this method to just `build` - we don't have any way yet to build a non-default manager anyway. The javadoc already says it builds a default configuration. ########## File path: solr/solr-ref-guide/src/circuit-breakers.adoc ########## @@ -0,0 +1,65 @@ += Circuit Breakers +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +Solr's circuit breaker infrastructure allows prevention of actions that can cause a node to go beyond its capacity or to go down. The +premise of circuit breakers is to ensure a higher quality of service and only accept request loads that are serviceable in the current +resource configuration. + +== When To Use Circuit Breakers +Circuit breakers should be used when the user wishes to trade request throughput for a higher Solr stability. If circuit breakers +are enabled, requests may be rejected under the condition of high node duress with an appropriate HTTP error code (typically 503). + +It is upto the client to handle the same and potentially build a retrial logic as this should ideally be a transient situation. + +== Circuit Breaker Configurations +The following flag controls the global activation/deactivation of circuit breakers. If this flag is disabled, all circuit breakers +will be disabled globally. Per circuit breaker configurations are specified in their respective sections later. + +[source,xml] +---- +<useCircuitBreakers>false</useCircuitBreakers> +---- + +== Currently Supported Circuit Breakers + +=== JVM Heap Usage Based Circuit Breaker +This circuit breaker tracks JVM heap memory usage and rejects incoming search requests with a 503 error code if the heap usage +exceeds a configured percentage of maximum heap allocated to the JVM (-XMx). The main configuration for this circuit breaker is +controlling the threshold percentage at which the breaker will trip. + +[source,xml] +---- +<memoryCircuitBreakerThreshold>75</memoryCircuitBreakerThreshold> +---- + +Consider the following example: + +JVM has been allocated a maximum heap of 5GB (-XMx) and memoryCircuitBreakerThreshold is set to 75. In this scenario, the heap usage Review comment: `-Xmx` ########## File path: solr/solr-ref-guide/src/circuit-breakers.adoc ########## @@ -0,0 +1,65 @@ += Circuit Breakers +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +Solr's circuit breaker infrastructure allows prevention of actions that can cause a node to go beyond its capacity or to go down. The +premise of circuit breakers is to ensure a higher quality of service and only accept request loads that are serviceable in the current +resource configuration. + +== When To Use Circuit Breakers +Circuit breakers should be used when the user wishes to trade request throughput for a higher Solr stability. If circuit breakers +are enabled, requests may be rejected under the condition of high node duress with an appropriate HTTP error code (typically 503). + +It is upto the client to handle the same and potentially build a retrial logic as this should ideally be a transient situation. + +== Circuit Breaker Configurations +The following flag controls the global activation/deactivation of circuit breakers. If this flag is disabled, all circuit breakers +will be disabled globally. Per circuit breaker configurations are specified in their respective sections later. + +[source,xml] +---- +<useCircuitBreakers>false</useCircuitBreakers> +---- + +== Currently Supported Circuit Breakers + +=== JVM Heap Usage Based Circuit Breaker +This circuit breaker tracks JVM heap memory usage and rejects incoming search requests with a 503 error code if the heap usage +exceeds a configured percentage of maximum heap allocated to the JVM (-XMx). The main configuration for this circuit breaker is +controlling the threshold percentage at which the breaker will trip. + +[source,xml] +---- +<memoryCircuitBreakerThreshold>75</memoryCircuitBreakerThreshold> +---- + +Consider the following example: + +JVM has been allocated a maximum heap of 5GB (-XMx) and memoryCircuitBreakerThreshold is set to 75. In this scenario, the heap usage +at which the circuit breaker will trip is 3.75GB. + +Note that this circuit breaker is checked for each incoming search request and considers the current heap usage of the node i.e every search +request will get the live heap usage and compare it against the set memory threshold. The check does not impact performance, +but any performance regressions that are suspected to be caused by this feature should be reported to the dev list. + + +== Performance Considerations +It is worth noting that while JVM circuit breaker does not add any noticeable overhead per query, having too many +circuit breakers checked for a single request can cause a performance overhead. + +In addition, it is a good practise to exponentially back off while retrying requests on a busy node. Review comment: practise -> practice ########## File path: solr/solr-ref-guide/src/circuit-breakers.adoc ########## @@ -0,0 +1,65 @@ += Circuit Breakers +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +Solr's circuit breaker infrastructure allows prevention of actions that can cause a node to go beyond its capacity or to go down. The +premise of circuit breakers is to ensure a higher quality of service and only accept request loads that are serviceable in the current +resource configuration. + +== When To Use Circuit Breakers +Circuit breakers should be used when the user wishes to trade request throughput for a higher Solr stability. If circuit breakers +are enabled, requests may be rejected under the condition of high node duress with an appropriate HTTP error code (typically 503). + +It is upto the client to handle the same and potentially build a retrial logic as this should ideally be a transient situation. + +== Circuit Breaker Configurations +The following flag controls the global activation/deactivation of circuit breakers. If this flag is disabled, all circuit breakers +will be disabled globally. Per circuit breaker configurations are specified in their respective sections later. + +[source,xml] +---- +<useCircuitBreakers>false</useCircuitBreakers> +---- + +== Currently Supported Circuit Breakers + +=== JVM Heap Usage Based Circuit Breaker +This circuit breaker tracks JVM heap memory usage and rejects incoming search requests with a 503 error code if the heap usage +exceeds a configured percentage of maximum heap allocated to the JVM (-XMx). The main configuration for this circuit breaker is +controlling the threshold percentage at which the breaker will trip. + +[source,xml] +---- +<memoryCircuitBreakerThreshold>75</memoryCircuitBreakerThreshold> +---- + +Consider the following example: + +JVM has been allocated a maximum heap of 5GB (-XMx) and memoryCircuitBreakerThreshold is set to 75. In this scenario, the heap usage +at which the circuit breaker will trip is 3.75GB. + +Note that this circuit breaker is checked for each incoming search request and considers the current heap usage of the node i.e every search Review comment: i.e -> , i.e. ########## File path: solr/solr-ref-guide/src/query-settings-in-solrconfig.adoc ########## @@ -170,6 +170,26 @@ This parameter sets the maximum number of documents to cache for any entry in th <queryResultMaxDocsCached>200</queryResultMaxDocsCached> ---- +=== useCircuitBreakers + +Global control flag for enabling circuit breakers Review comment: Add full stop. ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.solr.core.SolrConfig; + +/** + * Manages all registered circuit breaker instances. Responsible for a holistic view + * of whether a circuit breaker has tripped or not. + * + * There are two typical ways of using this class's instance: + * 1. Check if any circuit breaker has triggered -- and know which circuit breaker has triggered. + * 2. Get an instance of a specific circuit breaker and perform checks. + * + * It is a good practice to register new circuit breakers here if you want them checked for every + * request. + * + * NOTE: The current way of registering new default circuit breakers is minimal and not a long term + * solution. There will be a follow up with a SIP for a schema API design. + */ +public class CircuitBreakerManager { + + private final Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>(); + + // Allows replacing of existing circuit breaker + public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) { + circuitBreakerMap.put(circuitBreakerType, circuitBreaker); + } + + public CircuitBreaker getCircuitBreaker(CircuitBreakerType circuitBreakerType) { + assert circuitBreakerType != null; + + return circuitBreakerMap.get(circuitBreakerType); + } + + /** + * Check if any circuit breaker has triggered. + * @return CircuitBreakers which have triggered, null otherwise + */ + public Map<CircuitBreakerType, CircuitBreaker> checkedTrippedCircuitBreakers() { + Map<CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null; + + for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) { + CircuitBreaker circuitBreaker = entry.getValue(); + + if (circuitBreaker.isEnabled() && + circuitBreaker.isTripped()) { + if (triggeredCircuitBreakers == null) { + triggeredCircuitBreakers = new HashMap<>(); + } + + triggeredCircuitBreakers.put(entry.getKey(), circuitBreaker); + } + } + + return triggeredCircuitBreakers; + } + + /** + * Returns true if *any* circuit breaker has triggered, false if none have triggered + * + * NOTE: This method short circuits the checking of circuit breakers -- the method will + * return as soon as it finds a circuit breaker that is enabled and has triggered + */ + public boolean checkAnyCircuitBreakerTripped() { + for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) { + CircuitBreaker circuitBreaker = entry.getValue(); + + if (circuitBreaker.isEnabled() && + circuitBreaker.isTripped()) { + return true; + } + } + + return false; + } + + /** + * Construct the final error message to be printed when circuit breakers trip + * @param circuitBreakerMap Input list for circuit breakers + * @return Constructed error message + */ + public static String constructFinalErrorMessageString(Map<CircuitBreakerType, CircuitBreaker> circuitBreakerMap) { + assert circuitBreakerMap != null; + + StringBuilder sb = new StringBuilder(); + + for (CircuitBreakerType circuitBreakerType : circuitBreakerMap.keySet()) { + sb.append(circuitBreakerType.toString() + " " + circuitBreakerMap.get(circuitBreakerType).getDebugInfo()); Review comment: New-lines or other separator characters would make this message more readable, now we're getting a single very long line. ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java ########## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import org.apache.solr.core.SolrConfig; + +/** + * Default class to define circuit breakers for Solr. + * + * There are two (typical) ways to use circuit breakers: + * 1. Have them checked at admission control by default (use CircuitBreakerManager for the same) + * 2. Use the circuit breaker in a specific code path(s) + * + * TODO: This class should be grown as the scope of circuit breakers grow. + */ +public abstract class CircuitBreaker { + public static final String NAME = "circuitbreaker"; + + protected final SolrConfig solrConfig; + + public CircuitBreaker(SolrConfig solrConfig) { + this.solrConfig = solrConfig; + } + + // Global config for all circuit breakers. For specific circuit breaker configs, define + // your own config + protected boolean isEnabled() { + return solrConfig.useCircuitBreakers; + } + + /** + * Check if this request will trigger circuit breaker. Review comment: This javadoc is confusing - why not simply say "Check if this breaker is tripped"? It's not the request that trips the breaker, it's the underlying condition that does this. ########## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java ########## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import org.apache.solr.core.SolrConfig; + +/** + * Default class to define circuit breakers for Solr. + * + * There are two (typical) ways to use circuit breakers: + * 1. Have them checked at admission control by default (use CircuitBreakerManager for the same) + * 2. Use the circuit breaker in a specific code path(s) + * + * TODO: This class should be grown as the scope of circuit breakers grow. + */ +public abstract class CircuitBreaker { + public static final String NAME = "circuitbreaker"; + + protected final SolrConfig solrConfig; + + public CircuitBreaker(SolrConfig solrConfig) { + this.solrConfig = solrConfig; + } + + // Global config for all circuit breakers. For specific circuit breaker configs, define + // your own config + protected boolean isEnabled() { + return solrConfig.useCircuitBreakers; + } + + /** + * Check if this request will trigger circuit breaker. + */ + public abstract boolean isTripped(); + + /** + * Print debug useful info Review comment: Get useful debug info. ########## File path: solr/solr-ref-guide/src/query-settings-in-solrconfig.adoc ########## @@ -170,6 +170,26 @@ This parameter sets the maximum number of documents to cache for any entry in th <queryResultMaxDocsCached>200</queryResultMaxDocsCached> ---- +=== useCircuitBreakers + +Global control flag for enabling circuit breakers + +[source,xml] +---- +<useCircuitBreakers>true</useCircuitBreakers> +---- + +=== memoryCircuitBreakerThreshold + +Memory threshold in percentage for JVM heap usage defined in percentage of maximum heap allocated +to the JVM (-XMx). Ideally, this value should be in the range of 75-80% of maximum heap allocated Review comment: `-Xmx` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
