There's an interesting pattern called a "Skeptic" presented in this paper:

http://www.hpl.hp.com/techreports/Compaq-DEC/SRC-RR-77.pdf

(as seen on Werner Vogel's blog). It's somewhat similar to a CircuitBreaker but has different recovery behavior and perhaps deals better with intermittent problems. This would make an interesting addition to JRugged.

Hello There,

It would be extremely handy to get statistics out of a performance monitor going directly into Carbon/Graphite like Coda Hale's metrics library does - https://github.com/codahale/metrics.

Thanks,
MBL

For sub projects, upgrade things like Guava and other dependencies to the latest versions.

Timeouts are a useful stability pattern, however many high-level APIs omit the ability to set a timeout.

It would be nice if JRugged could provide a solution that allows existing services to be timeboxed.

If the wrapped service takes longer than a predefined timeLimit, the wrapper would throw some sort TimeoutException.

Also, after a timeLimit has been exceed, there should be some (optional) mechanism that attempts to cancel the wrapped service call (perhaps via sending an Interrupt, or a more explicit cancel() method defined by a JRugged Cancellable.cancel() interface).

It would be very nice to include some documentation that illustrates an example of how a Timeboxer could be used to wrap HttpClient calls, such that HttpRequests are nicely aborted after the timeLimit is exceeded.

The wrapper probably needs to expose mechanisms to control the threading model used to execute the Futures that the wrapper service will use internally. Perhaps this is as simple as allowing a custom ExecutorService to be injected into the wrapper.

An optional, but potentially useful, feature could be to allow several services to be simultaneously wrapped in the same timebox. For example: i have a composite service that assembles data from multiple different backends, if any of them haven't finished by the end of the timeLimit, i want to grab the results from the services that did finish and cancel the calls that weren't fast enough.

It would be great to be able to use the jrugged namespace (http://code.google.com/p/jrugged/wiki/SpringNamespaceExample) to configure jrugged with the recent annotation based configuration.

We have an interesting mix of 'long' and 'double' values being returned in the PerformanceMonitors. The Success/Failure latencies are longs in millis. The Averages are doubles (also in millis).

We should standardize on one or the other.

In class org.fishwife.jrugged.CircuitBreaker this can be done in trip() method. openCount.getAndIncrement() should be perform only if previous state was not OPEN.

When @CIRCUITBREAKER and @PerformanceMonitor are used together (or with other aspects), we need to impose a certain execution order. Currently, both aspects are unordered and an order cannot be specified.

A simple way to do this is to have them implement Ordered interface and also provide a setter for customization.

Is it possible to deploy the 3.2.1 artifacts to maven central? Thanks!

All links under 'Latest Java Doc' and 'Latest Stable Release' results in error.

Eg:

curl http://jrugged.s3.amazonaws.com/jrugged-core-3.1.0/index.html<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>AllAccessDisabled</Code><Message>All access to this object has been disabled</Message><RequestId>9A3BA658BB0C5CDE</RequestId><HostId>wcViigppx9DXVokLWICEGOsrivjWmVnKJanlv+1SRaj1v4QwOwcOAvjQEJ/72B+YHoQ54qDJikE=</HostId></Error>

When some circuit breaker is down,
If I enable CircuitBreakerAspect debug log,
CircuitBreakerAspect: line 108 call circuiteBreaker.getStatus() and getServiceStatus().
getServiceStatus() call allowRequest() and allowRequest() call canAttempt() and it change isAttempLive to true and circuitbreaker status broken.

Test Code

@Test
public void circuitBreakerRecovery() throws InterruptedException {
    for (int i = 0; i <= 3; i++) {
        try {
            bean.doSomething2();
            fail("raise exception");
        }
        catch (IllegalArgumentException ex) {
            // I want it 
        }
    }

    try {
        bean.doSomething2();
    }
    catch (CircuitBreakerException ex) {
        log.info("Now circuit opened");
    }

    Thread.sleep(2000L);
    bean.setError(false);
    log.info("Change  no more error in service");
    bean.doSomething2();
    assertTrue("No execption here", true);
}

Fixture

@CircuitBreaker(name = "cb2", limit = 3, resetMillis = 100L, windowMillis = 100L)
public void doSomething2() {
    if (error) {
        throw new IllegalArgumentException();
    }
}  

We have been using the library in production and found a few issues with the CircuitBreaker class implementation not being thread safe.

Looking at the source, I believe there are a number of concurrency issues with the class around the manipulation of state, isAttemptLive, isHardTrip, byPass fields. From my experience it looks like it's not thread safe, although hard to describe why. Here is an attempt to describe what we observed in a production scenario:

On occasion when the caller is highly concurrent (i.e. runs on many threads) the CircuitBreaker get's stuck in a HALF_CLOSED state. It's stuck in a HALF_CLOSED state because the isAttemptLive field is not atomic and is flipped in both the close, trip methods (when called from a successful or a unsuccessful invoke method) while at the same time (or out of order) the synchronized canAttempt also manipulates the isAttemptLive state.

It's possible for the close or trip methods to be called out of order to the synchronized canAttempt method. This can cause a situation where sate=HALF_CLOSED, isAttemptLive=true but the actual request has completed. Thus the isAttemptLive=true will never be flipped back as there is not attempt being made. The result is that the CB remains forever in a HALF_CLOSED state. I have verified existence of this situation of CircuitBreaker being in this state once in a debugging situation (jrugged-core version = 3.2.2).

For Jrugged-spring it would be good to be able to carry a jrugged version that is built against spring 4 and a version that is built against spring 5. Enable a way to have JRugged built and deployed against both.

Consider the following circuit breaker usage:

    @CircuitBreaker(name = "sameCircuitBreaker", windowMillis = "30", limit = "3")
    public void someMethod1() {

    }

    //... elsewhere in my code
    @CircuitBreaker(name = "sameCircuitBreaker", windowMillis = "60", limit = "10")
    public void someMethod2() {

    }

It's not determinate which of the configurations will actually be used for your app (break if 10 errors in 60ms or 3 errors in 30?). Since the first configuration will be used to instantiate the default interpreter for my circuit breaker and subsequent usages will look up the CB by name without modifying the interpreter, you can't know which config will actually be used in production. See https://github.com/Comcast/jrugged/blob/master/jrugged-aspects/src/main/java/org/fishwife/jrugged/aspects/CircuitBreakerAspect.java#L87

This could maybe be fixed if this configuration for the interpreter for each named circuit breaker was set only once separately from these circuit breaker annotations.

For the Last{Minute,Hour,Day} metrics (e.g. 99thPercentileSuccessLatencyLastMinute), if there has not been a sample in the specified time period a org.fishwife.jrugged.SampledQuantile$EmptySampleSetException is thrown when the value is requested.

This can be confusing. Some monitoring tools (SevOne, for example) will list a '--' value for the metric in this case. Also, causing even more confusion, monitoring tools will sometimes store the last known value and keep using it if a new value cannot be retrieved...resulting in a graph showing a steady value over time when there are actually no calls happening.

I believe that the Last{Minute,Hour,Day} stats should return 0 if there have been no matching requests. Perhaps we should only use the 'EmptySampleSetException' if there has been no data at all (ever)? The other alternative would be to have them report 0 if there have been no samples.

I tried getting the examples to run as documented in https://github.com/Comcast/jrugged/wiki/Running-Example-App but I got an error

When I try to mvn jetty:run in the jrugged-examples directory it fails and the error is:

	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:547)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:476)
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:302)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:298)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:193)

From googling around this seems to be a spring version issue.

Given the following test code:

import org.fishwife.jrugged.CircuitBreaker;
import org.fishwife.jrugged.CircuitBreakerException;

import java.util.concurrent.Callable;

public class Main {

    private static Boolean fail = true;

    public static void main(String[] args) {
        CircuitBreaker cb = new CircuitBreaker();
        for (int i = 0; i < 5; i++) {
            try {
                System.out.println(cb.getStatus());
                cb.invoke(new Callable<Object>() {
                    public Object call() throws Exception {
                        System.out.println("Made it to call()");
                        if (fail) {
                            fail = false;
                            throw new Exception();
                        }
                        System.out.println("call() passed");
                        return null;
                    };
                });
            } catch(CircuitBreakerException e) {
                System.out.println("CircuitBreakerException");
                try {
                    Thread.sleep(10000);
                } catch(InterruptedException ie) {}
            } catch (Exception e) {
                System.out.println("Exception");
            }
        }
    }
}

I get this result in the current version (3.2.2)

UP
Made it to call()
Exception
DOWN
CircuitBreakerException
DOWN
CircuitBreakerException
DEGRADED
CircuitBreakerException
DEGRADED
CircuitBreakerException

But I get this in the older version (3.1.1):

UP
Made it to call()
Exception
DOWN
CircuitBreakerException
DOWN
CircuitBreakerException
DEGRADED
Made it to call()
call() passed
UP
Made it to call()
call() passed

I was under the impression that the HALF_CLOSED (or DEGRADED) status would allow a single call to go through, but it appears to always be throwing a CircuitBreakerException before the call is made. Is there something I'm missing in order to get a CircuitBreaker in a HALF_CLOSED state to re-attempt a call?

If a RolledUpMonitoredService has a status that is not UP then getReasons can be used to determine the cause. Calling getReasons() enumerates the reason for each child service that is not UP. In many cases the child services are CircuitBreakers.

The current behavior for getReasons() lists only 'Open' or 'Send Probe Request' for each CircuitBreaker. It does not include any identification for which CircuitBreaker is affected.

It would be better to add the service name as a prefix for each reason, making it clear which service is causing the Non-UP status for the RolledUpMonitoredService.

Before Change: getReasons() might return
{ "Open", "Send Probe Request" }

After Chnage: getReasons() might return
{ "Service 1:Open", "Service 2:Send Probe Request" }

Hi,

I've successfully built JRugged using Java 1.7. The only fix needed was to update the AspectJ dependencies -- for Java 7, AspectJ version 1.7.2 is needed, so in jrugged-aspects/pom.xml we need to update aspectjrt and aspectjweaver artifacts to version 1.7.2.

Can you include this change in mainline?

Thanks,
Danijel

The performance monitor as it stands takes a sampled quantile from the total number of observed samples enabling the system to not have to store ALL the available samples but a much smaller number. This can potentially be inaccurate.

A wiki page to explain how to set up a project with the newly introduced annotation driven monitors would be very useful.

The HALF_CLOSED documentation states "... the CircuitBreaker will transition to a HALF_CLOSED state, where one call is allowed to go through as a test. If that call succeeds, the CircuitBreaker moves back to the CLOSED state; if it fails, it moves back to the OPEN state for another cooldown period". The implementation actually closes the CircuitBreaker if the attempt fails in the HALF_CLOSED state, thereby forcing the FailureInterpreter to start from scratch. This violates the one call promise if the CircuitBreaker is configured to have a limit > 0.

This code from CircuitBreaker appears to be at fault:

private void handleFailure(Throwable cause) throws Exception {
    if (failureInterpreter == null ||
        failureInterpreter.shouldTrip(cause)) {
            this.tripException = cause;
            trip();
    }

    if (isAttemptLive) {
        close(); // <--- shouldn't this be trip(); ?
    }

    if (cause instanceof Exception) {
        throw (Exception)cause;
    } else if (cause instanceof Error) {
        throw (Error)cause;
    } else {
        throw (RuntimeException)cause;
    }
}

Should all the uses of System.currentTimeMillis be replaced with SystemClock.currentTimeMillis? I would suppose the meaning of SystemClock is to allow tests to mock it up and simulate slow services and the like.

Of course, this would mean to allow each of the affected classes to receive a Clock as parameter. If not, the SystemClock is used.

The documentation on http://code.google.com/p/jrugged/wiki/SpringNamespaceExample shows an XSD schema purportedly located at http://www.fishwife.org/schema/jrugged/jrugged.xsd. However, that file doesn't exist there, and as there isn't actually a webserver running at www.fishwife.org, this causes schema validation to time out horribly in IDEs that attempt validation.

If we are going to have an XSD, let's find a better place to host it than a nonexistent webserver. :)

https://github.com/Comcast/jrugged/wiki/Running-Example-App

Has old instructions in it. It says
"TODO Alert*: These steps need to be updated somewhat, as the examples are now a sub-project of the mainline codebase. However, the steps are similar in spirit to what's below."

i'd like to update those

If the Initializer stops due to exceeding the maximum number of retries, there appears to be no API to detect that situation.

What steps will reproduce the problem?
As per the wiki page https://github.com/Comcast/jrugged/wiki/Spring-Namespace-Example, I can use property place holder for jrugged:perfmon attribute to turn performance monitoring On/Off by just making changes in property file. However, spring doesn't support BeanFactoryPostProcessor for namespaced attributes and tags. Thus leaving properties referred as ${propertyKey} as it is when the PerformanceMonitor bean created in Decorator.

What is the expected output? What do you see instead?
${propertyName} in xml configuration for jrugged:perfmon or jrugged:methods should be replaced with "propertyValue" configured in properties file using Post processor. Or, this deficiency should be be noted in the wiki page.

What version of the product are you using? On what operating system?
Jrugged 2.4 (core and spring)

Please provide any additional information below.

Right now if you look at a circuit breaker in Jrugged, the breaker itself has some convenience methods for retrieving and configuring values into the breaker. This is problematic because the behaviour of the breaker itself is actually governed by a FailureInterpreter class which lives inside of the breaker. This FailureInterpreter may have different configuration requirements than the default implementation making those CircuitBreaker convenience methods no good and lead to exceptions, errors and other miscommunications of understanding. Come up with a way to extract the Breaker/FailureInterpreter configuration so that they are not 'shared' as they are currently.