[GitHub] [solr] janhoy commented on a change in pull request #193: SOLR-15474: Support Pluggable Circuit Breakers

[GitBox <gi...@apache.org>]

janhoy commented on a change in pull request #193:
URL: https://github.com/apache/solr/pull/193#discussion_r734099124



##########
File path: solr/solr-ref-guide/src/circuit-breakers.adoc
##########
@@ -27,23 +27,16 @@ If circuit breakers are enabled, requests may be rejected under the condition of
 It is up to the client to handle this error and potentially build a retrial logic as this should ideally be a transient situation.
 
 == Circuit Breaker Configurations
-All circuit breaker configurations are listed in the `<circuitBreaker>` tags in `solrconfig.xml` as shown below:
+All circuit breaker configurations are listed as independent `<circuitBreaker>` entries in `solrconfig.xml` as shown below:
 
 [source,xml]
 ----
-<circuitBreaker class="solr.CircuitBreakerManager" enabled="true">
-  <!-- All specific configs in this section -->
+<circuitBreaker class="org.apache.solr.util.circuitbreaker.CPUCircuitBreaker" enable="true">
+  <int  name="threshold">75</int>
 </circuitBreaker>
 ----
 
-The `enabled` attribute 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.
-
-This attribute acts as the highest authority and global controller of circuit breakers.
-For using specific circuit breakers, each one needs to be individually enabled in addition to this flag being enabled.
-
-`CircuitBreakerManager` is the default manager for all circuit breakers that should be defined in the tag unless the user wishes to use a custom implementation.
+Each circuit breaker can be independently enabled/disabled using the corresponding flag.

Review comment:
       > I would disagree on the proposed XML format
   
   Is it the presence of an `eneable` attribute you don't like or the example variable substitution to make the attribute contollable from a sysProp? I think that was just an example from Chrstine of how things are often done in solr's config. But for this I doubt that it will even be added to the default solrconfig example at all? So let's just agree that the default is enabled and you can disable it with the xml attribute, and not with a child tag.




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org