You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/06/28 18:03:01 UTC
[GitHub] [lucene-solr] atris opened a new pull request #1626: SOLR-14588: Implement Circuit Breakers
atris opened a new pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626
This commit consists of two parts: add circuit breakers infrastructure and a "real" JVM heap memory based
circuit breaker which monitors incoming search requests and rejects them with SERVICE_TOO_BUSY error
if the defined threshold is breached, thus giving headroom to existing indexing and search requests
to complete.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447157028
##########
File path: solr/contrib/prometheus-exporter/src/test-files/solr/collection1/conf/solrconfig.xml
##########
@@ -83,6 +83,10 @@
<queryResultMaxDocsCached>200</queryResultMaxDocsCached>
+ <useCircuitBreakers>false</useCircuitBreakers>
+
+ <memoryCircuitBreakerThreshold>100</memoryCircuitBreakerThreshold>
Review comment:
Agreed. Removed the unnecessary config changes and added a new section, thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#issuecomment-652328056
Thank you @sigram !
@ErickErickson provided his approval offline (thanks Erick!) so I am going to merge this tomorrow morning 9 am my time (UTC +5.30). If any objections, please speak up.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r448496902
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##########
@@ -0,0 +1,111 @@
+/*
+ * 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.invoke.MethodHandles;
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * <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 memoryCircuitBreakerThresholdPct
+ * in solrconfig.xml.
+ * </p>
+ */
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ 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 getDebugInfo()
+ private final ThreadLocal<Long> seenMemory = new ThreadLocal<>();
+ private final ThreadLocal<Long> allowedMemory = new ThreadLocal<>();
+
+ public MemoryCircuitBreaker(SolrConfig solrConfig) {
+ super(solrConfig);
+
+ long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ if (currentMaxHeap <= 0) {
+ throw new IllegalArgumentException("Invalid JVM state for the max heap usage");
+ }
+
+ int thresholdValueInPercentage = solrConfig.memoryCircuitBreakerThresholdPct;
+ double thresholdInFraction = thresholdValueInPercentage / (double) 100;
+ heapMemoryThreshold = (long) (currentMaxHeap * thresholdInFraction);
+
+ if (heapMemoryThreshold <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or equal to zero");
+ }
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration of time
+ // after the circuit breaker condition is matched. This will optimize for per call
+ // overhead of calculating the condition parameters but can result in false positives.
+ @Override
+ public boolean isTripped() {
+ if (!isEnabled()) {
+ return false;
+ }
+
+ allowedMemory.set(getCurrentMemoryThreshold());
+
+ seenMemory.set(calculateLiveMemoryUsage());
+
+ return (seenMemory.get() >= allowedMemory.get());
Review comment:
It would be a two step process -- setting local value and then setting the thread local. Seems cleaner this way? I doubt if it will really cause a performance issue -- the variables are thread local
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447185389
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.SolrCore;
+
+/**
+ * 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.
+ *
+ * The memory threshold is defined as a percentage of the maximum memory allocated -- see memoryCircuitBreakerThreshold
+ * in solrconfig.xml
+ */
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN = ManagementFactory.getMemoryMXBean();
+
+ private final long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ // Assumption -- the value of these parameters will be set correctly before invoking printDebugInfo()
+ private ThreadLocal<Long> seenMemory = new ThreadLocal<>();
+ private ThreadLocal<Long> allowedMemory = new ThreadLocal<>();
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+ super(solrCore);
+
+ if (currentMaxHeap <= 0) {
+ throw new IllegalArgumentException("Invalid JVM state for the max heap usage");
+ }
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration of time
+ // after the circuit breaker condition is matched. This will optimize for per call
+ // overhead of calculating the condition parameters but can result in false positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+ if (!isCircuitBreakerEnabled()) {
+ return false;
+ }
+
+ allowedMemory.set(getCurrentMemoryThreshold());
+
+ seenMemory.set(calculateLiveMemoryUsage());
+
+ return (seenMemory.get() >= allowedMemory.get());
+ }
+
+ @Override
+ public String printDebugInfo() {
+ return "seenMemory=" + seenMemory.get() + " allowedMemory=" + allowedMemory.get();
+ }
+
+ private long getCurrentMemoryThreshold() {
+ int thresholdValueInPercentage = solrCore.getSolrConfig().memoryCircuitBreakerThreshold;
+ double thresholdInFraction = thresholdValueInPercentage / (double) 100;
+ long actualLimit = (long) (currentMaxHeap * thresholdInFraction);
+
+ if (actualLimit <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or equal to zero");
+ }
+
+ return actualLimit;
+ }
+
+ /**
+ * Calculate the live memory usage for the system. This method has package visibility
+ * to allow using for testing
+ * @return Memory usage in bytes
+ */
+ protected long calculateLiveMemoryUsage() {
+ // NOTE: MemoryUsageGaugeSet provides memory usage statistics but we do not use them
+ // here since MemoryUsageGaugeSet provides combination of heap and non heap usage and
Review comment:
Better wording, thanks!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ErickErickson commented on pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
ErickErickson commented on pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#issuecomment-651073728
Atri:
DebugComponentTest is failing a lot, although not reproducibly. See:
https://jenkins.thetaphi.de/job/Lucene-Solr-master-Linux/27171/
I took a brief look and it looks like, under some circumstances, the
circuitbreaker info is getting in the debug output throwing the test off
as it doesn’t expect the circuitbreaker data.
> On Jun 28, 2020, at 2:14 PM, Atri Sharma <no...@github.com> wrote:
>
>
> @atris requested your review on: #1626 SOLR-14588: Implement Circuit Breakers.
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub, or unsubscribe.
>
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r446908349
##########
File path: solr/contrib/prometheus-exporter/src/test-files/solr/collection1/conf/solrconfig.xml
##########
@@ -83,6 +83,10 @@
<queryResultMaxDocsCached>200</queryResultMaxDocsCached>
+ <useCircuitBreakers>false</useCircuitBreakers>
+
+ <memoryCircuitBreakerThreshold>100</memoryCircuitBreakerThreshold>
Review comment:
This is probably not needed in configs that don't actually use it (when useCircuitBreakers=false)?
Also, to make it more future-proof, we could put these in a section - the expectation is that we will have at least one more (CPU breaker) and potentially other ones too, so instead of adding these new breakers as new elements at this level we could add them as a section (as sub-elements).
##########
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.
+ *
Review comment:
The following probably belongs to the SIP ... but the way I think about the common usage of this class for different code-paths is if breaker configs are labeled and correspond to different code-paths, eg.:
* "query" -> one config
* "index" -> another config
* "foobar" -> yet another config, used perhaps in my custom component
Current implementation limits us to use the same config for potentially very different code paths.
##########
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() {
Review comment:
OMG, what a name :) Maybe just `checkTrippedBreakers` ?
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -804,6 +813,14 @@ private void initLibs(SolrResourceLoader loader, boolean isConfigsetTrusted) {
loader.reloadLuceneSPI();
}
+ private void validateMemoryBreakerThreshold() {
+ if (useCircuitBreakers) {
+ if (memoryCircuitBreakerThreshold > 100 || memoryCircuitBreakerThreshold < 0) {
+ throw new IllegalArgumentException("memoryCircuitBreakerThreshold is not a valid percentage");
Review comment:
I think 0 also doesn't make much sense.
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.SolrCore;
+
+/**
+ * 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.
+ *
+ * The memory threshold is defined as a percentage of the maximum memory allocated -- see memoryCircuitBreakerThreshold
+ * in solrconfig.xml
+ */
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN = ManagementFactory.getMemoryMXBean();
+
+ private final long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ // Assumption -- the value of these parameters will be set correctly before invoking printDebugInfo()
+ private ThreadLocal<Long> seenMemory = new ThreadLocal<>();
+ private ThreadLocal<Long> allowedMemory = new ThreadLocal<>();
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+ super(solrCore);
+
+ if (currentMaxHeap <= 0) {
+ throw new IllegalArgumentException("Invalid JVM state for the max heap usage");
+ }
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration of time
+ // after the circuit breaker condition is matched. This will optimize for per call
+ // overhead of calculating the condition parameters but can result in false positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+ if (!isCircuitBreakerEnabled()) {
+ return false;
+ }
+
+ allowedMemory.set(getCurrentMemoryThreshold());
+
+ seenMemory.set(calculateLiveMemoryUsage());
+
+ return (seenMemory.get() >= allowedMemory.get());
+ }
+
+ @Override
+ public String printDebugInfo() {
+ return "seenMemory=" + seenMemory.get() + " allowedMemory=" + allowedMemory.get();
+ }
+
+ private long getCurrentMemoryThreshold() {
+ int thresholdValueInPercentage = solrCore.getSolrConfig().memoryCircuitBreakerThreshold;
+ double thresholdInFraction = thresholdValueInPercentage / (double) 100;
Review comment:
This can be calculated once in the constructor - IIRC if SolrConfig is updated the core is reloaded anyway, which will construct the breaker once again.
##########
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.SolrCore;
+
+/**
+ * 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 SolrCore solrCore;
+
+ public CircuitBreaker(SolrCore solrCore) {
+ this.solrCore = solrCore;
+ }
+
+ // Global config for all circuit breakers. For specific circuit breaker configs, define
+ // your own config
+ protected boolean isCircuitBreakerEnabled() {
Review comment:
Maybe `isEnabled` ?
##########
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.SolrCore;
+
+/**
+ * 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 SolrCore solrCore;
+
+ public CircuitBreaker(SolrCore solrCore) {
+ this.solrCore = solrCore;
+ }
+
+ // Global config for all circuit breakers. For specific circuit breaker configs, define
+ // your own config
+ protected boolean isCircuitBreakerEnabled() {
+ return solrCore.getSolrConfig().useCircuitBreakers;
+ }
+
+ /**
+ * Check if this allocation will trigger circuit breaker.
+ */
+ public abstract boolean isCircuitBreakerGauntletTripped();
+
+ /**
+ * Print debug useful info
+ */
+ public abstract String printDebugInfo();
Review comment:
This doesn't actually print anything, maybe name it `getDebugInfo` ?
##########
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.SolrCore;
+
+/**
+ * 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 SolrCore solrCore;
+
+ public CircuitBreaker(SolrCore solrCore) {
+ this.solrCore = solrCore;
+ }
+
+ // Global config for all circuit breakers. For specific circuit breaker configs, define
+ // your own config
+ protected boolean isCircuitBreakerEnabled() {
+ return solrCore.getSolrConfig().useCircuitBreakers;
+ }
+
+ /**
+ * Check if this allocation will trigger circuit breaker.
+ */
+ public abstract boolean isCircuitBreakerGauntletTripped();
Review comment:
Maybe `isTripped` ?
##########
File path: solr/solr-ref-guide/src/index.adoc
##########
@@ -121,6 +122,8 @@ The *<<getting-started.adoc#getting-started,Getting Started>>* section guides yo
*<<solrcloud.adoc#solrcloud,SolrCloud>>*: This section describes SolrCloud, which provides comprehensive distributed capabilities.
*<<legacy-scaling-and-distribution.adoc#legacy-scaling-and-distribution,Legacy Scaling and Distribution>>*: This section tells you how to grow a Solr distribution by dividing a large index into sections called shards, which are then distributed across multiple servers, or by replicating a single index across multiple services.
+
+*<<circuit-breakers.adoc#circuit-breakers,Circuit Breakers>>*: This section talks about circuit breakers, a comprehensive way of allowing a higher stability of Solr nodes and predictability of request execution.
Review comment:
Erhm .. "a comprehensive way" it is not, but it is certainly "a way" :) I would also argue that it doesn't increase predictability of request execution because it introduces a new failure mode that clients have to handle - rather it's a way to ensure the service-level guarantees for requests that are accepted for execution.
##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -0,0 +1,81 @@
+= 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.
+
+== Types Of Circuit Breakers
+Circuit breakers can be of two types:
+
+=== Admission Control Checks
+
+Circuit breakers that are checked at admission control (request handlers). These circuit breakers are typically attached to a set
+of requests that check them before proceeding with the request. Example is JVM heap usage based circuit breaker (described below).
+
+For these type of circuit breakers, it is a good idea to register them with CircuitBreakerManager
+(org.apache.solr.util.circuitbreaker.CircuitBreakerManager) to allow a holistic check at the required admission control point.
+
+=== Custom Events/Code Paths Checks
+
+Circuit breakers that are needed only in special events or code paths.
+
+
+== 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 (-XMax). The main configuration for this circuit breaker is
+controlling the threshold percentage at which the JVM will trip.
Review comment:
"the JVM" -> "the breaker".
##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -0,0 +1,81 @@
+= 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.
+
+== Types Of Circuit Breakers
+Circuit breakers can be of two types:
+
+=== Admission Control Checks
+
+Circuit breakers that are checked at admission control (request handlers). These circuit breakers are typically attached to a set
+of requests that check them before proceeding with the request. Example is JVM heap usage based circuit breaker (described below).
+
+For these type of circuit breakers, it is a good idea to register them with CircuitBreakerManager
+(org.apache.solr.util.circuitbreaker.CircuitBreakerManager) to allow a holistic check at the required admission control point.
+
+=== Custom Events/Code Paths Checks
+
+Circuit breakers that are needed only in special events or code paths.
+
+
+== 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 (-XMax). The main configuration for this circuit breaker is
+controlling the threshold percentage at which the JVM will trip.
+
+[source,xml]
+----
+<memoryCircuitBreakerThreshold>75</memoryCircuitBreakerThreshold>
+----
+
+Consider the following example:
+
+JVM has been allocated a maximum heap of 5GB (-XMax) 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 live state of the node i.e every search
Review comment:
"Live state" in SolrCloud terminology is a loaded term ... perhaps it's better to simply use the "current heap usage".
##########
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;
+
+ for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) {
+ CircuitBreaker circuitBreaker = entry.getValue();
+
+ if (circuitBreaker.isCircuitBreakerEnabled() &&
+ circuitBreaker.isCircuitBreakerGauntletTripped()) {
+ 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 checkAllCircuitBreakers() {
+ for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) {
+ CircuitBreaker circuitBreaker = entry.getValue();
+
+ if (circuitBreaker.isCircuitBreakerEnabled() &&
+ circuitBreaker.isCircuitBreakerGauntletTripped()) {
+ 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).printDebugInfo());
+ }
+
+ 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(SolrCore solrCore) {
Review comment:
Do we actually need `SolrCore` here, or just the breakers' config (currently in `SolrConfig`)?
##########
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 (-XMax).
Review comment:
-Xmx
##########
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;
+
+ for (Map.Entry<CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) {
+ CircuitBreaker circuitBreaker = entry.getValue();
+
+ if (circuitBreaker.isCircuitBreakerEnabled() &&
+ circuitBreaker.isCircuitBreakerGauntletTripped()) {
+ 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 checkAllCircuitBreakers() {
Review comment:
Maybe `checkAnyBreakerTripped` ? Because we don't actually check all breakers here.
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.SolrCore;
+
+/**
+ * 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.
+ *
+ * The memory threshold is defined as a percentage of the maximum memory allocated -- see memoryCircuitBreakerThreshold
+ * in solrconfig.xml
+ */
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN = ManagementFactory.getMemoryMXBean();
+
+ private final long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ // Assumption -- the value of these parameters will be set correctly before invoking printDebugInfo()
+ private ThreadLocal<Long> seenMemory = new ThreadLocal<>();
+ private ThreadLocal<Long> allowedMemory = new ThreadLocal<>();
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+ super(solrCore);
+
+ if (currentMaxHeap <= 0) {
+ throw new IllegalArgumentException("Invalid JVM state for the max heap usage");
+ }
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration of time
+ // after the circuit breaker condition is matched. This will optimize for per call
+ // overhead of calculating the condition parameters but can result in false positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+ if (!isCircuitBreakerEnabled()) {
+ return false;
+ }
+
+ allowedMemory.set(getCurrentMemoryThreshold());
+
+ seenMemory.set(calculateLiveMemoryUsage());
+
+ return (seenMemory.get() >= allowedMemory.get());
+ }
+
+ @Override
+ public String printDebugInfo() {
+ return "seenMemory=" + seenMemory.get() + " allowedMemory=" + allowedMemory.get();
+ }
+
+ private long getCurrentMemoryThreshold() {
+ int thresholdValueInPercentage = solrCore.getSolrConfig().memoryCircuitBreakerThreshold;
+ double thresholdInFraction = thresholdValueInPercentage / (double) 100;
+ long actualLimit = (long) (currentMaxHeap * thresholdInFraction);
+
+ if (actualLimit <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or equal to zero");
+ }
+
+ return actualLimit;
+ }
+
+ /**
+ * Calculate the live memory usage for the system. This method has package visibility
+ * to allow using for testing
+ * @return Memory usage in bytes
+ */
+ protected long calculateLiveMemoryUsage() {
+ // NOTE: MemoryUsageGaugeSet provides memory usage statistics but we do not use them
+ // here since MemoryUsageGaugeSet provides combination of heap and non heap usage and
Review comment:
This comment is somewhat misleading ... if I correctly understand the intent :) MemoryUsageGaugeSet does provide the heap and the non-heap usages separately, so it's possible to get the value we want from it - but it incurs unnecessary cost and additional allocations, so we can do it cheaper by using MemoryMXBean directly.
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.SolrCore;
+
+/**
+ * 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.
+ *
+ * The memory threshold is defined as a percentage of the maximum memory allocated -- see memoryCircuitBreakerThreshold
Review comment:
It needs <p> tags to actually make a new paragraph.
##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -0,0 +1,81 @@
+= 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.
+
+== Types Of Circuit Breakers
+Circuit breakers can be of two types:
Review comment:
I don't think this distinction merits a separate section - these are not actually different types of breakers, they are just different usage scenarios, and only one is currently implemented.
##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -0,0 +1,81 @@
+= 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.
+
+== Types Of Circuit Breakers
+Circuit breakers can be of two types:
+
+=== Admission Control Checks
+
+Circuit breakers that are checked at admission control (request handlers). These circuit breakers are typically attached to a set
+of requests that check them before proceeding with the request. Example is JVM heap usage based circuit breaker (described below).
+
+For these type of circuit breakers, it is a good idea to register them with CircuitBreakerManager
+(org.apache.solr.util.circuitbreaker.CircuitBreakerManager) to allow a holistic check at the required admission control point.
+
+=== Custom Events/Code Paths Checks
+
+Circuit breakers that are needed only in special events or code paths.
+
+
+== 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 (-XMax). The main configuration for this circuit breaker is
Review comment:
The JVM arg is `-Xmx`
##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -0,0 +1,81 @@
+= 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.
+
+== Types Of Circuit Breakers
+Circuit breakers can be of two types:
+
+=== Admission Control Checks
+
+Circuit breakers that are checked at admission control (request handlers). These circuit breakers are typically attached to a set
+of requests that check them before proceeding with the request. Example is JVM heap usage based circuit breaker (described below).
+
+For these type of circuit breakers, it is a good idea to register them with CircuitBreakerManager
+(org.apache.solr.util.circuitbreaker.CircuitBreakerManager) to allow a holistic check at the required admission control point.
Review comment:
The doc should mention somewhere that this is currently the only scenario supported by default.
##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -0,0 +1,81 @@
+= 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.
+
+== Types Of Circuit Breakers
+Circuit breakers can be of two types:
+
+=== Admission Control Checks
+
+Circuit breakers that are checked at admission control (request handlers). These circuit breakers are typically attached to a set
+of requests that check them before proceeding with the request. Example is JVM heap usage based circuit breaker (described below).
+
+For these type of circuit breakers, it is a good idea to register them with CircuitBreakerManager
+(org.apache.solr.util.circuitbreaker.CircuitBreakerManager) to allow a holistic check at the required admission control point.
+
+=== Custom Events/Code Paths Checks
+
+Circuit breakers that are needed only in special events or code paths.
+
+
+== 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 (-XMax). The main configuration for this circuit breaker is
+controlling the threshold percentage at which the JVM will trip.
+
+[source,xml]
+----
+<memoryCircuitBreakerThreshold>75</memoryCircuitBreakerThreshold>
+----
+
+Consider the following example:
+
+JVM has been allocated a maximum heap of 5GB (-XMax) and memoryCircuitBreakerThreshold is set to 75. In this scenario, the heap usage
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447189990
##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -0,0 +1,81 @@
+= 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.
+
+== Types Of Circuit Breakers
+Circuit breakers can be of two types:
+
+=== Admission Control Checks
+
+Circuit breakers that are checked at admission control (request handlers). These circuit breakers are typically attached to a set
+of requests that check them before proceeding with the request. Example is JVM heap usage based circuit breaker (described below).
+
+For these type of circuit breakers, it is a good idea to register them with CircuitBreakerManager
+(org.apache.solr.util.circuitbreaker.CircuitBreakerManager) to allow a holistic check at the required admission control point.
Review comment:
I removed the section completely -- didnt make a lot of sense in user docs
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r448460273
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void register(CircuitBreaker.CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) {
+ circuitBreakerMap.put(circuitBreakerType, circuitBreaker);
+ }
+
+ public CircuitBreaker getCircuitBreaker(CircuitBreaker.CircuitBreakerType circuitBreakerType) {
+ assert circuitBreakerType != null;
+
+ return circuitBreakerMap.get(circuitBreakerType);
+ }
+
+ /**
+ * Check and return circuit breakers that have triggered
+ * @return CircuitBreakers which have triggered, null otherwise.
+ */
+ public Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> checkTripped() {
+ Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null;
+
+ for (Map.Entry<CircuitBreaker.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.
+ *
+ * <p>
+ * 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.
+ * </p>
+ */
+ public boolean checkAnyTripped() {
+ for (Map.Entry<CircuitBreaker.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 toErrorMessage(Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap) {
+ assert circuitBreakerMap != null;
+
+ StringBuilder sb = new StringBuilder();
+
+ for (CircuitBreaker.CircuitBreakerType circuitBreakerType : circuitBreakerMap.keySet()) {
+ sb.append(circuitBreakerType.toString() + " " + circuitBreakerMap.get(circuitBreakerType).getDebugInfo() + "\n");
Review comment:
do multiple append calls instead of string concat.
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ * <p>
+ * 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.
+ * </p>
+ */
+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 circuit breaker is tripped.
+ */
+ public abstract boolean isTripped();
+
+ /**
+ * Get debug useful info.
+ */
+ public abstract String getDebugInfo();
+
+ /**
+ * Types of circuit breakers.
+ */
+ public enum CircuitBreakerType {
+ MEMORY,
+ CPU
Review comment:
This is unused? Maybe delete it for now and add it back when somebody comes up with an implementation?
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -1164,6 +1168,16 @@ private SolrCoreMetricManager initCoreMetricManager(SolrConfig config) {
return coreMetricManager;
}
+ private CircuitBreakerManager initCircuitBreakerManager() {
+ if (solrConfig.useCircuitBreakers) {
+ CircuitBreakerManager circuitBreakerManager = CircuitBreakerManager.build(solrConfig);
+
+ return circuitBreakerManager;
+ }
+
+ return null;
Review comment:
This is a little scary that we sometimes return null here? I think I would prefer an implementation of CBM that knows it is turned off so we don't accidentally get an NPE somewhere else.
##########
File path: solr/core/src/test/org/apache/solr/util/TestCircuitBreaker.java
##########
@@ -0,0 +1,225 @@
+/*
+ * 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;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.SolrConfig;
+import org.apache.solr.search.QueryParsing;
+import org.apache.solr.util.circuitbreaker.CircuitBreaker;
+import org.apache.solr.util.circuitbreaker.MemoryCircuitBreaker;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TestRule;
+
+public class TestCircuitBreaker extends SolrTestCaseJ4 {
+ private final static int NUM_DOCS = 20;
+
+ @Rule
+ public TestRule solrTestRules = RuleChain.outerRule(new SystemPropertiesRestoreRule());
+
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+ System.setProperty("filterCache.enabled", "false");
+ System.setProperty("queryResultCache.enabled", "false");
+ System.setProperty("documentCache.enabled", "true");
+
+ initCore("solrconfig-memory-circuitbreaker.xml", "schema.xml");
+ for (int i = 0 ; i < NUM_DOCS ; i ++) {
+ assertU(adoc("name", "john smith", "id", "1"));
+ assertU(adoc("name", "johathon smith", "id", "2"));
+ assertU(adoc("name", "john percival smith", "id", "3"));
+ assertU(adoc("id", "1", "title", "this is a title.", "inStock_b1", "true"));
+ assertU(adoc("id", "2", "title", "this is another title.", "inStock_b1", "true"));
+ assertU(adoc("id", "3", "title", "Mary had a little lamb.", "inStock_b1", "false"));
+
+ //commit inside the loop to get multiple segments to make search as realistic as possible
+ assertU(commit());
+ }
+ }
+
+ @Override
+ public void tearDown() throws Exception {
+ super.tearDown();
+ }
+
+ public void testCBAlwaysTrips() throws IOException {
+ HashMap<String, String> args = new HashMap<String, String>();
+
+ args.put(QueryParsing.DEFTYPE, CircuitBreaker.NAME);
+ args.put(CommonParams.FL, "id");
+
+ CircuitBreaker circuitBreaker = new MockCircuitBreaker(h.getCore().getSolrConfig());
+
+ h.getCore().getCircuitBreakerManager().register(CircuitBreaker.CircuitBreakerType.MEMORY, circuitBreaker);
+
+ expectThrows(SolrException.class, () -> {
+ h.query(req("name:\"john smith\""));
+ });
+
+ circuitBreaker = new MemoryCircuitBreaker(h.getCore().getSolrConfig());
+
+ h.getCore().getCircuitBreakerManager().register(CircuitBreaker.CircuitBreakerType.MEMORY, circuitBreaker);
+ }
+
+ public void testCBFakeMemoryPressure() throws IOException {
+ HashMap<String, String> args = new HashMap<String, String>();
+
+ args.put(QueryParsing.DEFTYPE, CircuitBreaker.NAME);
+ args.put(CommonParams.FL, "id");
+
+ CircuitBreaker circuitBreaker = new FakeMemoryPressureCircuitBreaker(h.getCore().getSolrConfig());
+
+ h.getCore().getCircuitBreakerManager().register(CircuitBreaker.CircuitBreakerType.MEMORY, circuitBreaker);
+
+ expectThrows(SolrException.class, () -> {
+ h.query(req("name:\"john smith\""));
+ });
+
+ circuitBreaker = new MemoryCircuitBreaker(h.getCore().getSolrConfig());
+
+ h.getCore().getCircuitBreakerManager().register(CircuitBreaker.CircuitBreakerType.MEMORY, circuitBreaker);
Review comment:
What is this testing? Is this just resetting the circuitbreakermanager? Should be in a Before/After blocks?
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
Review comment:
This doesn't seem right... I can only have one circuit breaker of each type? I can imagine having multiple breakers that all work in conjunction - limit QPS to X, and limit QPM to Y, for example.
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void register(CircuitBreaker.CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) {
+ circuitBreakerMap.put(circuitBreakerType, circuitBreaker);
+ }
+
+ public CircuitBreaker getCircuitBreaker(CircuitBreaker.CircuitBreakerType circuitBreakerType) {
+ assert circuitBreakerType != null;
+
+ return circuitBreakerMap.get(circuitBreakerType);
+ }
+
+ /**
+ * Check and return circuit breakers that have triggered
+ * @return CircuitBreakers which have triggered, null otherwise.
+ */
+ public Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> checkTripped() {
+ Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null;
+
+ for (Map.Entry<CircuitBreaker.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.
+ *
+ * <p>
+ * 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.
+ * </p>
+ */
+ public boolean checkAnyTripped() {
Review comment:
This is unused?
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##########
@@ -0,0 +1,111 @@
+/*
+ * 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.invoke.MethodHandles;
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * <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 memoryCircuitBreakerThresholdPct
+ * in solrconfig.xml.
+ * </p>
+ */
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ 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 getDebugInfo()
+ private final ThreadLocal<Long> seenMemory = new ThreadLocal<>();
+ private final ThreadLocal<Long> allowedMemory = new ThreadLocal<>();
+
+ public MemoryCircuitBreaker(SolrConfig solrConfig) {
+ super(solrConfig);
+
+ long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ if (currentMaxHeap <= 0) {
+ throw new IllegalArgumentException("Invalid JVM state for the max heap usage");
+ }
+
+ int thresholdValueInPercentage = solrConfig.memoryCircuitBreakerThresholdPct;
+ double thresholdInFraction = thresholdValueInPercentage / (double) 100;
+ heapMemoryThreshold = (long) (currentMaxHeap * thresholdInFraction);
+
+ if (heapMemoryThreshold <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or equal to zero");
+ }
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration of time
+ // after the circuit breaker condition is matched. This will optimize for per call
+ // overhead of calculating the condition parameters but can result in false positives.
+ @Override
+ public boolean isTripped() {
+ if (!isEnabled()) {
+ return false;
+ }
+
+ allowedMemory.set(getCurrentMemoryThreshold());
+
+ seenMemory.set(calculateLiveMemoryUsage());
+
+ return (seenMemory.get() >= allowedMemory.get());
Review comment:
Should we store these to local values so that we don't have to call an extra `.get()` on the concurrent structure? Probably doesn't make a difference.
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void register(CircuitBreaker.CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) {
+ circuitBreakerMap.put(circuitBreakerType, circuitBreaker);
+ }
+
+ public CircuitBreaker getCircuitBreaker(CircuitBreaker.CircuitBreakerType circuitBreakerType) {
+ assert circuitBreakerType != null;
Review comment:
Is this needed?
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -224,6 +224,11 @@ private SolrConfig(SolrResourceLoader loader, String name, boolean isConfigsetTr
queryResultWindowSize = Math.max(1, getInt("query/queryResultWindowSize", 1));
queryResultMaxDocsCached = getInt("query/queryResultMaxDocsCached", Integer.MAX_VALUE);
enableLazyFieldLoading = getBool("query/enableLazyFieldLoading", false);
+
+ useCircuitBreakers = getBool("circuitBreaker/useCircuitBreakers", false);
+ memoryCircuitBreakerThresholdPct = getInt("circuitBreaker/memoryCircuitBreakerThresholdPct", 100);
Review comment:
I don't think 100 is a safe default here, since later we check that the value is between 50-95.
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void register(CircuitBreaker.CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) {
+ circuitBreakerMap.put(circuitBreakerType, circuitBreaker);
+ }
+
+ public CircuitBreaker getCircuitBreaker(CircuitBreaker.CircuitBreakerType circuitBreakerType) {
+ assert circuitBreakerType != null;
+
+ return circuitBreakerMap.get(circuitBreakerType);
+ }
+
+ /**
+ * Check and return circuit breakers that have triggered
+ * @return CircuitBreakers which have triggered, null otherwise.
+ */
+ public Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> checkTripped() {
+ Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null;
+
+ for (Map.Entry<CircuitBreaker.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.
+ *
+ * <p>
+ * 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.
+ * </p>
+ */
+ public boolean checkAnyTripped() {
+ for (Map.Entry<CircuitBreaker.CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) {
Review comment:
circuitBreakerMap.values()
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void register(CircuitBreaker.CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) {
+ circuitBreakerMap.put(circuitBreakerType, circuitBreaker);
+ }
+
+ public CircuitBreaker getCircuitBreaker(CircuitBreaker.CircuitBreakerType circuitBreakerType) {
Review comment:
This method is never used?
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##########
@@ -0,0 +1,111 @@
+/*
+ * 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.invoke.MethodHandles;
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * <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 memoryCircuitBreakerThresholdPct
+ * in solrconfig.xml.
+ * </p>
+ */
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ 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 getDebugInfo()
+ private final ThreadLocal<Long> seenMemory = new ThreadLocal<>();
+ private final ThreadLocal<Long> allowedMemory = new ThreadLocal<>();
+
+ public MemoryCircuitBreaker(SolrConfig solrConfig) {
+ super(solrConfig);
+
+ long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ if (currentMaxHeap <= 0) {
+ throw new IllegalArgumentException("Invalid JVM state for the max heap usage");
+ }
+
+ int thresholdValueInPercentage = solrConfig.memoryCircuitBreakerThresholdPct;
+ double thresholdInFraction = thresholdValueInPercentage / (double) 100;
+ heapMemoryThreshold = (long) (currentMaxHeap * thresholdInFraction);
+
+ if (heapMemoryThreshold <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or equal to zero");
+ }
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration of time
+ // after the circuit breaker condition is matched. This will optimize for per call
+ // overhead of calculating the condition parameters but can result in false positives.
+ @Override
+ public boolean isTripped() {
+ if (!isEnabled()) {
+ return false;
+ }
+
+ allowedMemory.set(getCurrentMemoryThreshold());
+
+ seenMemory.set(calculateLiveMemoryUsage());
+
+ return (seenMemory.get() >= allowedMemory.get());
+ }
+
+ @Override
+ public String getDebugInfo() {
+ if (seenMemory.get() == 0.0 || allowedMemory.get() == 0.0) {
Review comment:
Why are we comparing a long to a floating point?
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ * <p>
+ * 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.
+ * </p>
+ */
+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 circuit breaker is tripped.
+ */
+ public abstract boolean isTripped();
+
+ /**
+ * Get debug useful info.
+ */
+ public abstract String getDebugInfo();
+
+ /**
+ * Types of circuit breakers.
+ */
+ public enum CircuitBreakerType {
Review comment:
Does this need a "CUSTOM" type or something else for folks who will be implementing their own?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r446935828
##########
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:
If this is called on every query request then it makes sense to avoid allocations until they are needed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#issuecomment-651182733
> Atri: DebugComponentTest is failing a lot, although not reproducibly. See: https://jenkins.thetaphi.de/job/Lucene-Solr-master-Linux/27171/ I took a brief look and it looks like, under some circumstances, the circuitbreaker info is getting in the debug output throwing the test off as it doesn’t expect the circuitbreaker data.
Jan pushed a fix to revert the last bits of tests from the earlier PR -- DebugComponentTest should be fine now (please let me know if otherwise).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r448495347
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void register(CircuitBreaker.CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) {
+ circuitBreakerMap.put(circuitBreakerType, circuitBreaker);
+ }
+
+ public CircuitBreaker getCircuitBreaker(CircuitBreaker.CircuitBreakerType circuitBreakerType) {
+ assert circuitBreakerType != null;
+
+ return circuitBreakerMap.get(circuitBreakerType);
+ }
+
+ /**
+ * Check and return circuit breakers that have triggered
+ * @return CircuitBreakers which have triggered, null otherwise.
+ */
+ public Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> checkTripped() {
+ Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null;
+
+ for (Map.Entry<CircuitBreaker.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.
+ *
+ * <p>
+ * 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.
+ * </p>
+ */
+ public boolean checkAnyTripped() {
+ for (Map.Entry<CircuitBreaker.CircuitBreakerType, CircuitBreaker> entry : circuitBreakerMap.entrySet()) {
Review comment:
Need both the key and the value in this loop, so values() wont work?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447048456
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -804,6 +813,14 @@ private void initLibs(SolrResourceLoader loader, boolean isConfigsetTrusted) {
loader.reloadLuceneSPI();
}
+ private void validateMemoryBreakerThreshold() {
+ if (useCircuitBreakers) {
+ if (memoryCircuitBreakerThreshold > 100 || memoryCircuitBreakerThreshold < 0) {
Review comment:
This is more of a sanity test to ensure that the incoming values lie in the range of percentage. If we wish to restrain the actual domain of values that memoryCircuitBreakerThreshold can take, I can add another check and add the same in the documentation.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r449135773
##########
File path: solr/solr-ref-guide/src/index.adoc
##########
@@ -10,6 +10,7 @@
streaming-expressions, \
solrcloud, \
legacy-scaling-and-distribution, \
+ circuit-breakers, \
Review comment:
FYI @ctargett I'm not sure if this page deserves its own page or should be a part of another... and if it is its own page as Atris did then where the logical placement is. Any way; I just want to bring this to your attention.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447094476
##########
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 prefer cleaner abstractions -- helps avoid compiler errors ;)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r448494462
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ * <p>
+ * 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.
+ * </p>
+ */
+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 circuit breaker is tripped.
+ */
+ public abstract boolean isTripped();
+
+ /**
+ * Get debug useful info.
+ */
+ public abstract String getDebugInfo();
+
+ /**
+ * Types of circuit breakers.
+ */
+ public enum CircuitBreakerType {
Review comment:
Preferably, they can add their own type when implementing a new one?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r448493962
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -224,6 +224,11 @@ private SolrConfig(SolrResourceLoader loader, String name, boolean isConfigsetTr
queryResultWindowSize = Math.max(1, getInt("query/queryResultWindowSize", 1));
queryResultMaxDocsCached = getInt("query/queryResultMaxDocsCached", Integer.MAX_VALUE);
enableLazyFieldLoading = getBool("query/enableLazyFieldLoading", false);
+
+ useCircuitBreakers = getBool("circuitBreaker/useCircuitBreakers", false);
+ memoryCircuitBreakerThresholdPct = getInt("circuitBreaker/memoryCircuitBreakerThresholdPct", 100);
Review comment:
Changed to 95.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r448550485
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##########
@@ -0,0 +1,111 @@
+/*
+ * 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.invoke.MethodHandles;
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * <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 memoryCircuitBreakerThresholdPct
+ * in solrconfig.xml.
+ * </p>
+ */
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ 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 getDebugInfo()
+ private final ThreadLocal<Long> seenMemory = new ThreadLocal<>();
+ private final ThreadLocal<Long> allowedMemory = new ThreadLocal<>();
+
+ public MemoryCircuitBreaker(SolrConfig solrConfig) {
+ super(solrConfig);
+
+ long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ if (currentMaxHeap <= 0) {
+ throw new IllegalArgumentException("Invalid JVM state for the max heap usage");
+ }
+
+ int thresholdValueInPercentage = solrConfig.memoryCircuitBreakerThresholdPct;
+ double thresholdInFraction = thresholdValueInPercentage / (double) 100;
+ heapMemoryThreshold = (long) (currentMaxHeap * thresholdInFraction);
+
+ if (heapMemoryThreshold <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or equal to zero");
+ }
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration of time
+ // after the circuit breaker condition is matched. This will optimize for per call
+ // overhead of calculating the condition parameters but can result in false positives.
+ @Override
+ public boolean isTripped() {
+ if (!isEnabled()) {
+ return false;
+ }
+
+ allowedMemory.set(getCurrentMemoryThreshold());
+
+ seenMemory.set(calculateLiveMemoryUsage());
+
+ return (seenMemory.get() >= allowedMemory.get());
Review comment:
this solution has an extra unboxing.
##########
File path: solr/core/src/test/org/apache/solr/util/TestCircuitBreaker.java
##########
@@ -0,0 +1,220 @@
+/*
+ * 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;
+
+import java.util.HashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.SolrConfig;
+import org.apache.solr.search.QueryParsing;
+import org.apache.solr.util.circuitbreaker.CircuitBreaker;
+import org.apache.solr.util.circuitbreaker.MemoryCircuitBreaker;
+import org.junit.After;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TestRule;
+
+public class TestCircuitBreaker extends SolrTestCaseJ4 {
+ private final static int NUM_DOCS = 20;
+
+ @Rule
+ public TestRule solrTestRules = RuleChain.outerRule(new SystemPropertiesRestoreRule());
+
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+ System.setProperty("filterCache.enabled", "false");
+ System.setProperty("queryResultCache.enabled", "false");
+ System.setProperty("documentCache.enabled", "true");
+
+ initCore("solrconfig-memory-circuitbreaker.xml", "schema.xml");
+ for (int i = 0 ; i < NUM_DOCS ; i ++) {
+ assertU(adoc("name", "john smith", "id", "1"));
+ assertU(adoc("name", "johathon smith", "id", "2"));
+ assertU(adoc("name", "john percival smith", "id", "3"));
+ assertU(adoc("id", "1", "title", "this is a title.", "inStock_b1", "true"));
+ assertU(adoc("id", "2", "title", "this is another title.", "inStock_b1", "true"));
+ assertU(adoc("id", "3", "title", "Mary had a little lamb.", "inStock_b1", "false"));
+
+ //commit inside the loop to get multiple segments to make search as realistic as possible
+ assertU(commit());
+ }
+ }
+
+ @Override
+ public void tearDown() throws Exception {
+ super.tearDown();
+ }
+
+ @After
+ public void after() {
+ CircuitBreaker circuitBreaker = new MemoryCircuitBreaker(h.getCore().getSolrConfig());
+
+ h.getCore().getCircuitBreakerManager().register(circuitBreaker);
Review comment:
This appends to the list, doesn't do any clearing. By the last test you'll have three of the same circuit breakers running.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#issuecomment-652821771
Thank you @anshumg ! You were indeed the final piece of the puzzle here :)
I am waiting for precommit to succeed and will merge. Thank you everybody for the review and iterations!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447597583
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -804,6 +813,14 @@ private void initLibs(SolrResourceLoader loader, boolean isConfigsetTrusted) {
loader.reloadLuceneSPI();
}
+ private void validateMemoryBreakerThreshold() {
+ if (useCircuitBreakers) {
+ if (memoryCircuitBreakerThreshold > 100 || memoryCircuitBreakerThreshold < 0) {
+ throw new IllegalArgumentException("memoryCircuitBreakerThreshold is not a valid percentage");
+ }
Review comment:
Added the same, thanks
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#issuecomment-651305132
@ErickErickson @sigram Thanks for reviewing. I have updated the PR per your comments. Unfortunately I had to force push due to the magnitude of removing the solrconfig.xml file changes.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#issuecomment-652569167
@madrob Updated. Please see and let me know your thoughts and comments.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
madrob commented on pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#issuecomment-652595688
Overall, I'm wondering if there gets to be a hysteresis condition where we are using enough memory to trigger the circuit breakers but not enough to trigger GC and the server gets into an unavailable state with no remedy. I don't know how we detect this or fix it, short of a restart.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris merged pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris merged pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ErickErickson commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
ErickErickson commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r446684636
##########
File path: solr/CHANGES.txt
##########
@@ -11,6 +11,7 @@ Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this r
New Features
---------------------
* SOLR-14440: Introduce new Certificate Authentication Plugin to load Principal from certificate subject. (Mike Drob)
+* SOLR-14588: Introduce Circuit Breaker Infrastructure and a JVM heap usage memory tracking circuit breaker implementation (Atri Sharma)
Review comment:
Minor nit, should be a new line between these.
##########
File path: solr/contrib/clustering/src/test-files/clustering/solr/collection1/conf/solrconfig.xml
##########
@@ -123,6 +123,17 @@
The purpose is to enable easy caching of user/application level data.
The regenerator argument should be specified as an implementation
of solr.search.CacheRegenerator if autowarming is desired. -->
+
+ <!-- Enable Circuit Breakers
+ -->
+ <useCircuitBreakers>false</useCircuitBreakers>
+
+ <!-- Memory Circuit Breaker Threshold In Percentage
+
+ Post this percentage usage of the heap, incoming queries will be rejected
+ -->
+ <memoryCircuitBreakerThreshold>100</memoryCircuitBreakerThreshold>
Review comment:
A little bit of guidance here would be helpful, maybe "70-80% heap usage is typical". Numbers are pulled out of thin air, just looking for a starting point for a newbie.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -804,6 +813,14 @@ private void initLibs(SolrResourceLoader loader, boolean isConfigsetTrusted) {
loader.reloadLuceneSPI();
}
+ private void validateMemoryBreakerThreshold() {
+ if (useCircuitBreakers) {
+ if (memoryCircuitBreakerThreshold > 100 || memoryCircuitBreakerThreshold < 0) {
Review comment:
Does it really make sense to allow either of these values? 0 seems like it'd cause everything to break (haven't seen the rest of the code yet, maybe 0 is a special case). 100% seems too late. Do we have any good information about what reasonable upper and lower bounds are? And should we enforce them? Say 50%/90% as a straw-man proposal for discussion...
##########
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:
This seems like an awfully small file, perhaps put this in the abstract CircuitBreaker class? No big deal either way.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -804,6 +813,14 @@ private void initLibs(SolrResourceLoader loader, boolean isConfigsetTrusted) {
loader.reloadLuceneSPI();
}
+ private void validateMemoryBreakerThreshold() {
+ if (useCircuitBreakers) {
+ if (memoryCircuitBreakerThreshold > 100 || memoryCircuitBreakerThreshold < 0) {
+ throw new IllegalArgumentException("memoryCircuitBreakerThreshold is not a valid percentage");
+ }
Review comment:
Add what valid percentages are here, especially if we decide to enforce as above..
This should also echo the number entered and what the valid limits are, something like:
memoryCircuitBreakerThreshold was set to" + memoryCircuitBreakerThreshold + ". Valid percentages must be between X% and Y%"
##########
File path: solr/contrib/prometheus-exporter/src/test-files/solr/collection1/conf/solrconfig.xml
##########
@@ -83,6 +83,10 @@
<queryResultMaxDocsCached>200</queryResultMaxDocsCached>
+ <useCircuitBreakers>false</useCircuitBreakers>
+
+ <memoryCircuitBreakerThreshold>100</memoryCircuitBreakerThreshold>
Review comment:
Same as above.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -522,6 +527,10 @@ public SolrRequestParsers getRequestParsers() {
public final int queryResultWindowSize;
public final int queryResultMaxDocsCached;
public final boolean enableLazyFieldLoading;
+
+ // Circuit Breaker Configuration
+ public final boolean useCircuitBreakers;
+ public final int memoryCircuitBreakerThreshold;
Review comment:
I always like to put in default values where the Java defaults wouldn't work, just in case "somehow" this isn't set in future. memoryCircuitBreakerThreshold defaults to 0 in this case, is that OK?
And maybe name this memoryCircuitBreakerThresholdPct?
##########
File path: solr/core/src/test-files/solr/collection1/conf/solrconfig-implicitproperties.xml
##########
@@ -42,6 +42,8 @@
<enableLazyFieldLoading>true</enableLazyFieldLoading>
<queryResultWindowSize>20</queryResultWindowSize>
<queryResultMaxDocsCached>20</queryResultMaxDocsCached>
+ <useCircuitBreakers>false</useCircuitBreakers>
Review comment:
Why is including the new circuitbreaker configs defaulting to false necessary? If they're left out, this should be a no-op, correct?
##########
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:
Why not just allocate a new HashMap here and avoid the null check below?
Hmmm, I suppose if there aren't any circuit breakers one could return null, NM.
##########
File path: solr/example/example-DIH/solr/db/conf/solrconfig.xml
##########
@@ -502,6 +502,16 @@
-->
<queryResultMaxDocsCached>200</queryResultMaxDocsCached>
+ <!-- Enable Circuit Breakers
+ -->
+ <useCircuitBreakers>false</useCircuitBreakers>
+
+ <!-- Memory Circuit Breaker Threshold In Percentage
+
Review comment:
I don't think there's any reason to include these in all these solrconfig files. I'd be sure they're include as _comments_ at least in server/solr/configsets/_default and sample_techproducts_configs but not elsewhere. Except, of course, the one you use for testing this functionality.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447093442
##########
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##########
@@ -804,6 +813,14 @@ private void initLibs(SolrResourceLoader loader, boolean isConfigsetTrusted) {
loader.reloadLuceneSPI();
}
+ private void validateMemoryBreakerThreshold() {
+ if (useCircuitBreakers) {
+ if (memoryCircuitBreakerThreshold > 100 || memoryCircuitBreakerThreshold < 0) {
+ throw new IllegalArgumentException("memoryCircuitBreakerThreshold is not a valid percentage");
+ }
Review comment:
Same as above. If we wish to enforce a smaller domain of values, we should discuss the range and I will add extra checks.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447188868
##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -0,0 +1,81 @@
+= 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.
+
+== Types Of Circuit Breakers
+Circuit breakers can be of two types:
Review comment:
Removed the same, thanks
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ErickErickson commented on pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
ErickErickson commented on pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#issuecomment-650812400
I left some comments, I need to look at the code more closely before I finish.
> On Jun 28, 2020, at 2:03 PM, Atri Sharma <no...@github.com> wrote:
>
> This commit consists of two parts: add circuit breakers infrastructure and a "real" JVM heap memory based
> circuit breaker which monitors incoming search requests and rejects them with SERVICE_TOO_BUSY error
> if the defined threshold is breached, thus giving headroom to existing indexing and search requests
> to complete.
>
> You can view, comment on, or merge this pull request online at:
>
> https://github.com/apache/lucene-solr/pull/1626
>
> Commit Summary
>
> • SOLR-14588: Implement Circuit Breakers
> File Changes
>
> • M solr/CHANGES.txt (1)
> • M solr/contrib/clustering/src/test-files/clustering/solr/collection1/conf/solrconfig.xml (11)
> • M solr/contrib/prometheus-exporter/src/test-files/solr/collection1/conf/solrconfig.xml (4)
> • M solr/core/src/java/org/apache/solr/core/SolrConfig.java (19)
> • M solr/core/src/java/org/apache/solr/core/SolrCore.java (16)
> • M solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java (32)
> • A solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java (55)
> • A solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java (128)
> • A solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerType.java (26)
> • A solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java (97)
> • M solr/core/src/resources/EditableSolrConfigAttributes.json (2)
> • M solr/core/src/test-files/solr/collection1/conf/solrconfig-cache-enable-disable.xml (4)
> • M solr/core/src/test-files/solr/collection1/conf/solrconfig-elevate.xml (4)
> • M solr/core/src/test-files/solr/collection1/conf/solrconfig-implicitproperties.xml (2)
> • A solr/core/src/test-files/solr/collection1/conf/solrconfig-memory-circuitbreaker.xml (91)
> • M solr/core/src/test-files/solr/collection1/conf/solrconfig-sortingresponse.xml (4)
> • M solr/core/src/test-files/solr/collection1/conf/solrconfig.xml (10)
> • M solr/core/src/test-files/solr/collection1/conf/solrconfig_perf.xml (2)
> • M solr/core/src/test/org/apache/solr/core/SolrCoreTest.java (2)
> • M solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java (2)
> • A solr/core/src/test/org/apache/solr/util/TestCircuitBreaker.java (227)
> • M solr/example/example-DIH/solr/db/conf/solrconfig.xml (10)
> • M solr/example/example-DIH/solr/mail/conf/solrconfig.xml (10)
> • M solr/example/example-DIH/solr/solr/conf/solrconfig.xml (10)
> • M solr/example/files/conf/solrconfig.xml (30)
> • M solr/server/solr/configsets/_default/conf/solrconfig.xml (10)
> • M solr/server/solr/configsets/sample_techproducts_configs/conf/solrconfig.xml (10)
> • A solr/solr-ref-guide/src/circuit-breakers.adoc (81)
> • M solr/solr-ref-guide/src/config-api.adoc (7)
> • M solr/solr-ref-guide/src/index.adoc (3)
> • M solr/solr-ref-guide/src/query-settings-in-solrconfig.adoc (20)
> Patch Links:
>
> • https://github.com/apache/lucene-solr/pull/1626.patch
> • https://github.com/apache/lucene-solr/pull/1626.diff
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or unsubscribe.
>
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447123680
##########
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.
+ *
Review comment:
Agreed. I added the single configuration since there is only one code path. I have a SIP upcoming to add an API for controlling these configs, will add this to the same. Thanks for bringing the same up.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447738671
##########
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:
Fixed, thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r447116490
##########
File path: solr/core/src/test-files/solr/collection1/conf/solrconfig-implicitproperties.xml
##########
@@ -42,6 +42,8 @@
<enableLazyFieldLoading>true</enableLazyFieldLoading>
<queryResultWindowSize>20</queryResultWindowSize>
<queryResultMaxDocsCached>20</queryResultMaxDocsCached>
+ <useCircuitBreakers>false</useCircuitBreakers>
Review comment:
Fair point, removed, thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r448495565
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void register(CircuitBreaker.CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) {
+ circuitBreakerMap.put(circuitBreakerType, circuitBreaker);
+ }
+
+ public CircuitBreaker getCircuitBreaker(CircuitBreaker.CircuitBreakerType circuitBreakerType) {
Review comment:
Added for sanity.
##########
File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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<CircuitBreaker.CircuitBreakerType, CircuitBreaker> circuitBreakerMap = new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void register(CircuitBreaker.CircuitBreakerType circuitBreakerType, CircuitBreaker circuitBreaker) {
+ circuitBreakerMap.put(circuitBreakerType, circuitBreaker);
+ }
+
+ public CircuitBreaker getCircuitBreaker(CircuitBreaker.CircuitBreakerType circuitBreakerType) {
+ assert circuitBreakerType != null;
+
+ return circuitBreakerMap.get(circuitBreakerType);
+ }
+
+ /**
+ * Check and return circuit breakers that have triggered
+ * @return CircuitBreakers which have triggered, null otherwise.
+ */
+ public Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> checkTripped() {
+ Map<CircuitBreaker.CircuitBreakerType, CircuitBreaker> triggeredCircuitBreakers = null;
+
+ for (Map.Entry<CircuitBreaker.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.
+ *
+ * <p>
+ * 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.
+ * </p>
+ */
+ public boolean checkAnyTripped() {
Review comment:
Added for sanity
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ctargett commented on a change in pull request #1626: SOLR-14588: Implement Circuit Breakers
Posted by GitBox <gi...@apache.org>.
ctargett commented on a change in pull request #1626:
URL: https://github.com/apache/lucene-solr/pull/1626#discussion_r451754651
##########
File path: solr/solr-ref-guide/src/index.adoc
##########
@@ -10,6 +10,7 @@
streaming-expressions, \
solrcloud, \
legacy-scaling-and-distribution, \
+ circuit-breakers, \
Review comment:
I missed this before it was committed - it shouldn't be a top-level page IMO. I'm not sure where to put it, and I'm also working on a major re-org of all pages, so I'll leave it for now and figure out where it should go as part of the re-org process.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org