Using shortcircuited in errorpercentage makes it hard to close circuit again

[JorritSalverda]

In HystrixCommandMetrics.java method getHealthCounts the errorpercentage is calculated as follows.

long totalCount = failure + success + timeout + threadPoolRejected + shortCircuited + semaphoreRejected;
long errorCount = failure + timeout + threadPoolRejected + shortCircuited + semaphoreRejected;
int errorPercentage = 0;

if (totalCount > 0) {
    errorPercentage = (int) ((double) errorCount / totalCount * 100);
}

To me it seems that using shortCircuited and both rejected values in the errorCount obscures the real error rate at the dependency itself, making it possible that as soon as the circuit breaker closes due to a successful canary call it closes again.

What's your experience with this?

[JorritSalverda]

Never mind, when rebuilding Hystrix in .net I forgot the essential resetCounters when circuit is closed :)

[mattrjacobs]

@JorritSalverda Yeah, the resetCounters is important :)

I think you raised a good point in the other train of thought. After considering it, it doesn't seem correct that SHORT_CIRCUITED outcomes should factor into a health percentage. I think in practice this shouldn't happen (due to resetCounters), but I think the definition should be clearer than only commands which actually attempted to run should count for/against a circuit's health.

WDYT?

[JorritSalverda]

I think it would depend on a fallback being executed successfully or an exception thrown instead whether it should count as an error. Executing the fallback is more of a warning if you compare it to the levels used in logging.

[mattrjacobs]

I'm not sure I follow your statement above. I think fallback success/error shouldn't factor into circuit breaker activity. I think that circuit breaker health should depend solely on success/failure of the command. The point I was trying to make is that a previous SHORT-CIRCUIT outcomes gives no indication about the present health of the command, just about the past state. Therefore, I don't believe it makes sense to include it in health calculations.

So, when I have time, I will issue a PR that drops SHORT-CIRCUITs from health calculations.

[JorritSalverda]

Yes you're right, it shouldn't be factored in regardless of fallback as it says nothing about the dependencies' health.

[mattrjacobs]

Merged #1034, so closing this issue. Thanks for bringing this up, @JorritSalverda