[ 
https://issues.apache.org/jira/browse/CAMEL-15928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17251979#comment-17251979
 ] 

Alex Liroyd commented on CAMEL-15928:
-------------------------------------

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)

Reply via email to