janhoy commented on code in PR #1871:
URL: https://github.com/apache/solr/pull/1871#discussion_r1315168456
##########
solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java:
##########
@@ -101,6 +112,30 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
}
+ /**
+ * Check if UPDATE circuit breakers are tripped. Override this method in sub
classes that do not
+ * want to check circuit breakers.
+ *
+ * @return true if circuit breakers are tripped, false otherwise.
+ */
+ protected boolean checkCircuitBreakers(SolrQueryRequest req,
SolrQueryResponse rsp) {
+ CircuitBreakerRegistry circuitBreakerRegistry =
req.getCore().getCircuitBreakerRegistry();
+ if (circuitBreakerRegistry.isEnabled(RequestType.UPDATE)) {
+ List<CircuitBreaker> trippedCircuitBreakers =
+ circuitBreakerRegistry.checkTripped(RequestType.UPDATE);
+ if (trippedCircuitBreakers != null) {
+ String errorMessage =
CircuitBreakerRegistry.toErrorMessage(trippedCircuitBreakers);
+ rsp.add(STATUS, FAILURE);
+ rsp.setException(
+ new SolrException(
+ SolrException.ErrorCode.SERVICE_UNAVAILABLE,
Review Comment:
I agree this sounds like a more appropritate code. Perhaps it has better
support in load balancers/gateways?
The existing CB for query uses 503 code, so we should perhaps not change the
code in 9.x.
So we could safely change to 429 in main branch and document it as a
breaking change. Or we could change it for the new pluggable CBs, but not for
the legacy `CircuitBreakerManager`, providing back compat for existing
users/configs. Wdyt?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
