Netflix / Hystrix

Hystrix is a latency and fault tolerance library designed to isolate points of access to remote systems, services and 3rd party libraries, stop cascading failure and enable resilience in complex distributed systems where failure is inevitable.
24.07k stars 4.7k forks source link

Concurrency Strategy fails for timeout case - ThreadLocals not bounded to timeout thread #1767

Open dmartinsid opened 6 years ago

dmartinsid commented 6 years ago

Hello guys,

I have a custom concurrency strategy that copy Spring RequestAttributes ThreadLocal to another Thread.

It's work very well for a case of Exception, fallback function is called OK, but for a timeout case when my Callable is called the concurrent thread doesn't have the Thread Locals and I get a exception from Spring. The problem seems that wrapCallable must be called before to set ThreadLocals to HystrixTimer Thread use it.

For now we will disable timeout feature, but would be nice that it works too for our threadlocal case:

Test case

package com.netflix.hystrix.timeout;

import static org.junit.Assert.assertEquals;

import org.junit.Before;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletWebRequest;

import com.netflix.config.ConfigurationManager;
import com.netflix.hystrix.strategy.HystrixPlugins;

public class CircuitBreakerThreadIsolationTimeoutTest {

    @Before
    public void setup() {
        RequestContextHolder.setRequestAttributes(new ServletWebRequest(new MockHttpServletRequest()));
        HystrixPlugins.getInstance().registerConcurrencyStrategy(new CustomConcurrencyStrategy());
        ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 1);
    }

    @Test
    public void mustCallFallbackOnTimeout() {
        String fallbackResponse = new MyHystrixCommand().execute();

        assertEquals("fallback", fallbackResponse);
    }
}

Custom Concurrent Strategy

package com.netflix.hystrix.timeout;

import java.util.concurrent.Callable;

import org.springframework.web.context.request.RequestContextHolder;

import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy;

public class CustomConcurrencyStrategy extends HystrixConcurrencyStrategy  {

    public <T> Callable<T> wrapCallable(final Callable<T> callable) {
        return new CustomCallable<>(callable, RequestContextHolder.currentRequestAttributes());
    }
}

Custom Callable

package com.netflix.hystrix.timeout;

import java.util.concurrent.Callable;

import org.springframework.web.context.request.RequestAttributes;
import org.springframework.web.context.request.RequestContextHolder;

public class CustomCallable<T> implements Callable<T> {

    private final Callable<T> callable;
    private final RequestAttributes requestAttributes;

    public CustomCallable(final Callable<T> callable, final RequestAttributes requestAttributes) {
        this.callable = callable;
        this.requestAttributes = requestAttributes;
    }

    @Override
    public T call() throws Exception {
        try {
            RequestContextHolder.setRequestAttributes(requestAttributes);
            return callable.call();
        } finally {
            RequestContextHolder.resetRequestAttributes();
        }
    }
}

My Command

package com.netflix.hystrix.timeout;

import com.netflix.hystrix.HystrixCommand;
import com.netflix.hystrix.HystrixCommandGroupKey;
import com.netflix.hystrix.HystrixCommandKey;
import com.netflix.hystrix.HystrixThreadPoolKey;

public class MyHystrixCommand extends HystrixCommand<String> {

    public MyHystrixCommand() {
        super(Setter
                .withGroupKey(HystrixCommandGroupKey.Factory.asKey("GROUP"))
                .andThreadPoolKey(HystrixThreadPoolKey.Factory.asKey("THREAD-POOL"))
                .andCommandKey(HystrixCommandKey.Factory.asKey("timeout-command")
                ));
    }

    @Override
    protected String run() throws Exception {
        Thread.sleep(5);
        return "must not be called";
    }

    @Override
    protected String getFallback() {
        return "fallback";
    }

}
kristiap commented 6 years ago

We have the same problem with spring sleuth tracing and hystrix.

If we use hystrix-core 1.5.11 it works and with 1.5.12 it fails. I think it is this commit that broke it https://github.com/Netflix/Hystrix/commit/951c6f9daa6dc469911b2e29a77fd85d90b4ab1e

The wrapCallable is being called on the timer thread in 1.5.12

whiskeysierra commented 6 years ago

Is there any progress on this? Apart from downgrading to 1.5.11, is there a workaround?

victormferrara commented 5 years ago

@whiskeysierra Maybe it's a bit overkill, but before finding this issue I implemented what's explained in #1653