[
https://issues.apache.org/jira/browse/CAMEL-15928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17251979#comment-17251979
]
Alex Liroyd edited comment on CAMEL-15928 at 12/18/20, 8:01 PM:
----------------------------------------------------------------
Please check attached patch: [^camel_fix_for_fallback.diff]
*Some explanation for fix:*
# The idea is the same : wrap in time-out, then wrap in circuit-breaker
# I had to remove - CircuitBreakerTimeoutTask, because fallback was not
triggered for regular exceptions
# processInCopy - it's just copy-paste, what u had in task
# throw exception is added to fallback(where we don't have registered any
fallback)
throw RuntimeExchangeException.wrapRuntimeException(throwable);
because, otherwise for case when CB is OPEN, I would not able to receive
target exception as result of route execution
Note: probably //exchange.setException(throwable); would be enough, but in
this case ResilienceRouteRejectedTest is failing, and I decided to not touch it
That's pretty much it.
*Cases which tested:*
# No fallback configured:
{code:java}
<route id="myRoute">
<from uri="myRouteID"/>
<circuitBreaker configurationRef="{{myCB}}">
<to uri="direct:myExecute"/>
</circuitBreaker>
</route>{code}
#
## Throw exception, which is not in "recordException" for CB
AR=ER - received exception on client, CB is not triggered
## Throw exception, which is in "recordException" for CB
AR=ER - received exception on client, CB is triggered
AR2=ER2 - if CB is open - get CB open exception right away
## Set time-out
### If time-out exception is in recorded for CB
AR=ER CB is triggered
### if time-out is not in recorded for CB
AR=ER CB is not triggered
# Fallback is configured:
{code:java}
<route id="myRoute">
<from uri="myRouteID"/>
<circuitBreaker configurationRef="{{myCB}}">
<to uri="direct:myExecute"/>
<onFallback>
<bean method="test" ref="myFallback"/>
</onFallback>
</circuitBreaker>
</route>{code}
#
## Throw exception, which is not in "recordException" for CB
AR=ER - CB is not triggered. Fallback executed
## Throw exception, which is in "recordException" for CB
AR=ER - CB is triggered. Fallback is executed
AR2=ER2 - if CB is open - initial call falls right away without any execution
+ fallback is executed
## Set time-out
### If time-out exception is in recorded for CB
AR=ER CB is triggered + fallback is executed
### if time-out is not in recorded for CB
AR=ER CB is not triggered, fallback is executed
Note: so basically fallback is executed for every case
[~davsclaus], please check patch and my explanation.
I really need this fix in 3.4.5. I will be able to create PR on Monday. Just
take a look, and say, what do you think. I basically tested all existing cases
+ change itself is not really huge.
was (Author: liroyd):
Please check attached patch: [^camel_fix_for_fallback.diff]
*Some explanation for fix:*
# The idea is the same : wrap in time-out, then wrap in circuit-breaker
# I had to remove - CircuitBreakerTimeoutTask, because fallback was not
triggered for regular exceptions
# processInCopy - it's just copy-paste, what u had in task
# throw exception is added to fallback(where we don't have registered any
fallback)
throw RuntimeExchangeException.wrapRuntimeException(throwable);
because, otherwise for case when CB is OPEN, I would not able to receive target
exception as result of route execution
Note: probably //exchange.setException(throwable); would be enough, but in this
case ResilienceRouteRejectedTest
That's pretty much it.
*Cases which tested:*
# No fallback configured:
{code:java}
<route id="myRoute">
<from uri="myRouteID"/>
<circuitBreaker configurationRef="{{myCB}}">
<to uri="direct:myExecute"/>
</circuitBreaker>
</route>{code}
## Throw exception, which is not in "recordException" for CB
AR=ER - received exception on client, CB is not triggered
## Throw exception, which is in "recordException" for CB
AR=ER - received exception on client, CB is triggered
AR2=ER2 - if CB is open - get CB open exception right away
## Set time-out
### If time-out exception is in recorded for CB
AR=ER CB is triggered
### if time-out is not in recorded for CB
AR=ER CB is not triggered
# Fallback is configured:
{code:java}
<route id="myRoute">
<from uri="myRouteID"/>
<circuitBreaker configurationRef="{{myCB}}">
<to uri="direct:myExecute"/>
<onFallback>
<bean method="test" ref="myFallback"/>
</onFallback>
</circuitBreaker>
</route>{code}
## Throw exception, which is not in "recordException" for CB
AR=ER - CB is not triggered. Fallback executed
## Throw exception, which is in "recordException" for CB
AR=ER - CB is triggered. Fallback is executed
AR2=ER2 - if CB is open - initial call falls right away without any execution +
fallback is executed
## Set time-out
### If time-out exception is in recorded for CB
AR=ER CB is triggered + fallback is executed
### if time-out is not in recorded for CB
AR=ER CB is not triggered, fallback is executed
Note: so basically fallback is executed for every case
[~davsclaus], please check patch and my explanation.
I really need this fix in 3.4.5. I will be able to create PR on Monday. Just
take a look, and say, what do you think. I basically tested all existing cases
+ change itself is not really huge.
> TimeoutException does not trigger Resilience4j circuit breaker
> --------------------------------------------------------------
>
> Key: CAMEL-15928
> URL: https://issues.apache.org/jira/browse/CAMEL-15928
> Project: Camel
> Issue Type: Improvement
> Components: came-core, eip
> Affects Versions: 3.4.4
> Reporter: Alex Liroyd
> Priority: Major
> Fix For: 3.8.0
>
> Attachments: camel_fix_for_fallback.diff, hot_fix.diff
>
>
> Currently Timeout exceptions does not trigger circuit breaker. But they
> should. I don't want to continue spam my server, if it slightly started dying.
> I tried to hot-fix in the next way - [^hot_fix.diff]
> The idea behind patch is next. Currently we wrap our call with circuit
> breaker and only after that with time limiter. So, circuit breaker doesn't
> know anything about time-outs.
> And basically I do opposite - initially wrap call with time limiter and only
> after that, wrap it with circuit breaker. So circuit breaker will aware about
> time-out exception and can react properly.
> The issue which I have afterward, that, for cases when circuit breaker was
> open, I started receiving blank 200 OK response.
> I tried to fix it by removing recover(fallbackTask) part at all:
>
> {code:java}
> // Try.ofCallable(task).recover(fallbackTask).andFinally(() ->
> callback.done(false)).get(); //old code
> Try.ofCallable(task).andFinally(() -> callback.done(false)).get(); // new
> line of code
> {code}
> And seems like it works fine. But tests are failing, and I'm not sure how
> exactly it should be fixed.
> Also another fix, which seems like works fine and tests are not failing:
> CircuitBreakerFallbackTask
>
> {code:java}
> } else if (throwable instanceof CallNotPermittedException) {
> // the circuit breaker triggered a call rejected
>
> exchange.setProperty(CircuitBreakerConstants.RESPONSE_SUCCESSFUL_EXECUTION,
> false);
> exchange.setProperty(CircuitBreakerConstants.RESPONSE_FROM_FALLBACK,
> false);
> exchange.setProperty(CircuitBreakerConstants.RESPONSE_SHORT_CIRCUITED,
> true);
> exchange.setProperty(CircuitBreakerConstants.RESPONSE_REJECTED, true);
> throw RuntimeExchangeException.wrapRuntimeException(throwable); // new
> line of code
> //return exchange; // old code
> {code}
>
>
> Please, assist.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
