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

[dmartinsid]

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]

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 951c6f9

The wrapCallable is being called on the timer thread in 1.5.12

[whiskeysierra]

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

[victormferrara]

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