Set hystrix timeout for Zuul routes based on whether the hystrix command timeout, default timeout, or ribbon timeouts are set. Fixes #2606 (#2633)
This commit is contained in:
@@ -17,6 +17,8 @@
|
||||
|
||||
package org.springframework.cloud.netflix.zuul.filters.route.support;
|
||||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration;
|
||||
import org.springframework.cloud.netflix.ribbon.RibbonHttpResponse;
|
||||
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
|
||||
@@ -28,6 +30,7 @@ import org.springframework.http.client.ClientHttpResponse;
|
||||
import com.netflix.client.AbstractLoadBalancerAwareClient;
|
||||
import com.netflix.client.ClientRequest;
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
import com.netflix.client.config.IClientConfigKey;
|
||||
import com.netflix.client.http.HttpResponse;
|
||||
import com.netflix.config.DynamicIntProperty;
|
||||
import com.netflix.config.DynamicPropertyFactory;
|
||||
@@ -46,6 +49,7 @@ import com.netflix.zuul.context.RequestContext;
|
||||
public abstract class AbstractRibbonCommand<LBC extends AbstractLoadBalancerAwareClient<RQ, RS>, RQ extends ClientRequest, RS extends HttpResponse>
|
||||
extends HystrixCommand<ClientHttpResponse> implements RibbonCommand {
|
||||
|
||||
private static final Log LOGGER = LogFactory.getLog(AbstractRibbonCommand.class);
|
||||
protected final LBC client;
|
||||
protected RibbonCommandContext context;
|
||||
protected ZuulFallbackProvider zuulFallbackProvider;
|
||||
@@ -70,7 +74,7 @@ public abstract class AbstractRibbonCommand<LBC extends AbstractLoadBalancerAwar
|
||||
public AbstractRibbonCommand(String commandKey, LBC client,
|
||||
RibbonCommandContext context, ZuulProperties zuulProperties,
|
||||
ZuulFallbackProvider fallbackProvider, IClientConfig config) {
|
||||
this(getSetter(commandKey, zuulProperties), client, context, fallbackProvider, config);
|
||||
this(getSetter(commandKey, zuulProperties, config), client, context, fallbackProvider, config);
|
||||
}
|
||||
|
||||
protected AbstractRibbonCommand(Setter setter, LBC client,
|
||||
@@ -83,16 +87,47 @@ public abstract class AbstractRibbonCommand<LBC extends AbstractLoadBalancerAwar
|
||||
this.config = config;
|
||||
}
|
||||
|
||||
protected static HystrixCommandProperties.Setter createSetter(IClientConfig config, String commandKey, ZuulProperties zuulProperties) {
|
||||
DynamicPropertyFactory dynamicPropertyFactory = DynamicPropertyFactory.getInstance();
|
||||
int defaultHystrixTimeout = dynamicPropertyFactory.getIntProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds",
|
||||
0).get();
|
||||
int commandHystrixTimeout = dynamicPropertyFactory.getIntProperty("hystrix.command." + commandKey + ".execution.isolation.thread.timeoutInMilliseconds",
|
||||
0).get();
|
||||
int ribbonReadTimeout = config == null ? RibbonClientConfiguration.DEFAULT_READ_TIMEOUT :
|
||||
config.get(IClientConfigKey.Keys.ReadTimeout, RibbonClientConfiguration.DEFAULT_READ_TIMEOUT).intValue();
|
||||
int ribbonConnectTimeout = config == null ? RibbonClientConfiguration.DEFAULT_CONNECT_TIMEOUT :
|
||||
config.get(IClientConfigKey.Keys.ConnectTimeout, RibbonClientConfiguration.DEFAULT_CONNECT_TIMEOUT).intValue();
|
||||
int ribbonTimeout = ribbonConnectTimeout + ribbonReadTimeout;
|
||||
int hystrixTimeout;
|
||||
if(commandHystrixTimeout > 0) {
|
||||
hystrixTimeout = commandHystrixTimeout;
|
||||
}
|
||||
else if( defaultHystrixTimeout > 0) {
|
||||
hystrixTimeout = defaultHystrixTimeout;
|
||||
} else {
|
||||
hystrixTimeout = ribbonTimeout;
|
||||
}
|
||||
if(hystrixTimeout < ribbonTimeout) {
|
||||
LOGGER.warn("The Hystrix timeout of " + hystrixTimeout + "ms for the command " + commandKey +
|
||||
" is set lower than the combination of the Ribbon read and connect timeout, " + ribbonTimeout + "ms.");
|
||||
}
|
||||
return HystrixCommandProperties.Setter().withExecutionIsolationStrategy(
|
||||
zuulProperties.getRibbonIsolationStrategy()).withExecutionTimeoutInMilliseconds(hystrixTimeout);
|
||||
}
|
||||
|
||||
@Deprecated
|
||||
//TODO remove in 2.0.x
|
||||
protected static Setter getSetter(final String commandKey, ZuulProperties zuulProperties) {
|
||||
return getSetter(commandKey, zuulProperties, null);
|
||||
}
|
||||
|
||||
protected static Setter getSetter(final String commandKey,
|
||||
ZuulProperties zuulProperties) {
|
||||
ZuulProperties zuulProperties, IClientConfig config) {
|
||||
|
||||
// @formatter:off
|
||||
Setter commandSetter = Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("RibbonCommand"))
|
||||
.andCommandKey(HystrixCommandKey.Factory.asKey(commandKey));
|
||||
|
||||
final HystrixCommandProperties.Setter setter = HystrixCommandProperties.Setter()
|
||||
.withExecutionIsolationStrategy(zuulProperties.getRibbonIsolationStrategy()).withExecutionTimeoutInMilliseconds(
|
||||
RibbonClientConfiguration.DEFAULT_CONNECT_TIMEOUT + RibbonClientConfiguration.DEFAULT_READ_TIMEOUT);
|
||||
final HystrixCommandProperties.Setter setter = createSetter(config, commandKey, zuulProperties);
|
||||
if (zuulProperties.getRibbonIsolationStrategy() == ExecutionIsolationStrategy.SEMAPHORE){
|
||||
final String name = ZuulConstants.ZUUL_EUREKA + commandKey + ".semaphore.maxSemaphores";
|
||||
// we want to default to semaphore-isolation since this wraps
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package org.springframework.cloud.netflix.zuul.filters.route.apache;
|
||||
|
||||
import java.util.HashSet;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
|
||||
@@ -11,6 +12,9 @@ import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider
|
||||
|
||||
import com.netflix.client.config.DefaultClientConfigImpl;
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
import com.netflix.client.config.IClientConfigKey;
|
||||
import com.netflix.config.ConfigurationManager;
|
||||
import com.netflix.hystrix.strategy.properties.HystrixPropertiesFactory;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.mockito.Matchers.anyString;
|
||||
@@ -39,6 +43,12 @@ public class HttpClientRibbonCommandFactoryTest {
|
||||
this.ribbonCommandFactory = new HttpClientRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet<ZuulFallbackProvider>());
|
||||
}
|
||||
|
||||
@After
|
||||
public void after() {
|
||||
ConfigurationManager.getConfigInstance().clear();
|
||||
HystrixPropertiesFactory.reset();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValue() throws Exception {
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
@@ -47,4 +57,50 @@ public class HttpClientRibbonCommandFactoryTest {
|
||||
assertEquals(2000, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValueSetting() throws Exception {
|
||||
ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 50);
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
doReturn("service").when(context).getServiceId();
|
||||
HttpClientRibbonCommand ribbonCommand = this.ribbonCommandFactory.create(context);
|
||||
assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValueCommandSetting() throws Exception {
|
||||
ConfigurationManager.getConfigInstance().setProperty("hystrix.command.service.execution.isolation.thread.timeoutInMilliseconds", 50);
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
doReturn("service").when(context).getServiceId();
|
||||
HttpClientRibbonCommand ribbonCommand = this.ribbonCommandFactory.create(context);
|
||||
assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValueCommandAndDefaultSetting() throws Exception {
|
||||
ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 30);
|
||||
ConfigurationManager.getConfigInstance().setProperty("hystrix.command.service.execution.isolation.thread.timeoutInMilliseconds", 50);
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
doReturn("service").when(context).getServiceId();
|
||||
HttpClientRibbonCommand ribbonCommand = this.ribbonCommandFactory.create(context);
|
||||
assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValueRibbonTimeouts() throws Exception {
|
||||
SpringClientFactory springClientFactory = mock(SpringClientFactory.class);
|
||||
ZuulProperties zuulProperties = new ZuulProperties();
|
||||
RibbonLoadBalancingHttpClient loadBalancingHttpClient = mock(RibbonLoadBalancingHttpClient.class);
|
||||
IClientConfig clientConfig = new DefaultClientConfigImpl();
|
||||
clientConfig.set(IClientConfigKey.Keys.ConnectTimeout, 100);
|
||||
clientConfig.set(IClientConfigKey.Keys.ReadTimeout, 500);
|
||||
doReturn(loadBalancingHttpClient).when(springClientFactory).getClient(anyString(),
|
||||
eq(RibbonLoadBalancingHttpClient.class));
|
||||
doReturn(clientConfig).when(springClientFactory).getClientConfig(anyString());
|
||||
HttpClientRibbonCommandFactory ribbonCommandFactory = new HttpClientRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet<ZuulFallbackProvider>());
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
doReturn("service").when(context).getServiceId();
|
||||
HttpClientRibbonCommand ribbonCommand = ribbonCommandFactory.create(context);
|
||||
assertEquals(600, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
package org.springframework.cloud.netflix.zuul.filters.route.okhttp;
|
||||
|
||||
import java.util.HashSet;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
|
||||
@@ -11,6 +12,9 @@ import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider
|
||||
|
||||
import com.netflix.client.config.DefaultClientConfigImpl;
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
import com.netflix.client.config.IClientConfigKey;
|
||||
import com.netflix.config.ConfigurationManager;
|
||||
import com.netflix.hystrix.strategy.properties.HystrixPropertiesFactory;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.mockito.Matchers.anyString;
|
||||
@@ -39,6 +43,12 @@ public class OkHttpRibbonCommandFactoryTest {
|
||||
commandFactory = new OkHttpRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet<ZuulFallbackProvider>());
|
||||
}
|
||||
|
||||
@After
|
||||
public void after() {
|
||||
ConfigurationManager.getConfigInstance().clear();
|
||||
HystrixPropertiesFactory.reset();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValue() throws Exception {
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
@@ -47,4 +57,50 @@ public class OkHttpRibbonCommandFactoryTest {
|
||||
assertEquals(2000, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValueSetting() throws Exception {
|
||||
ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 50);
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
doReturn("service").when(context).getServiceId();
|
||||
OkHttpRibbonCommand ribbonCommand = this.commandFactory.create(context);
|
||||
assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValueCommandSetting() throws Exception {
|
||||
ConfigurationManager.getConfigInstance().setProperty("hystrix.command.service.execution.isolation.thread.timeoutInMilliseconds", 50);
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
doReturn("service").when(context).getServiceId();
|
||||
OkHttpRibbonCommand ribbonCommand = this.commandFactory.create(context);
|
||||
assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValueCommandAndDefaultSetting() throws Exception {
|
||||
ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 30);
|
||||
ConfigurationManager.getConfigInstance().setProperty("hystrix.command.service.execution.isolation.thread.timeoutInMilliseconds", 50);
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
doReturn("service").when(context).getServiceId();
|
||||
OkHttpRibbonCommand ribbonCommand = this.commandFactory.create(context);
|
||||
assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixTimeoutValueRibbonTimeouts() throws Exception {
|
||||
SpringClientFactory springClientFactory = mock(SpringClientFactory.class);
|
||||
ZuulProperties zuulProperties = new ZuulProperties();
|
||||
OkHttpLoadBalancingClient loadBalancingHttpClient = mock(OkHttpLoadBalancingClient.class);
|
||||
IClientConfig clientConfig = new DefaultClientConfigImpl();
|
||||
clientConfig.set(IClientConfigKey.Keys.ConnectTimeout, 100);
|
||||
clientConfig.set(IClientConfigKey.Keys.ReadTimeout, 500);
|
||||
doReturn(loadBalancingHttpClient).when(springClientFactory).getClient(anyString(),
|
||||
eq(OkHttpLoadBalancingClient.class));
|
||||
doReturn(clientConfig).when(springClientFactory).getClientConfig(anyString());
|
||||
OkHttpRibbonCommandFactory commandFactory = new OkHttpRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet<ZuulFallbackProvider>());
|
||||
RibbonCommandContext context = mock(RibbonCommandContext.class);
|
||||
doReturn("service").when(context).getServiceId();
|
||||
OkHttpRibbonCommand ribbonCommand = commandFactory.create(context);
|
||||
assertEquals(600, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue());
|
||||
}
|
||||
|
||||
}
|
||||
@@ -27,6 +27,7 @@ import com.netflix.client.ClientException;
|
||||
import com.netflix.client.ClientRequest;
|
||||
import com.netflix.client.IResponse;
|
||||
import com.netflix.client.RequestSpecificRetryHandler;
|
||||
import com.netflix.client.config.DefaultClientConfigImpl;
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
import com.netflix.client.http.HttpResponse;
|
||||
import com.netflix.hystrix.HystrixCommandProperties;
|
||||
@@ -147,7 +148,7 @@ public class RibbonCommandCauseFallbackPropagationTest {
|
||||
AbstractLoadBalancerAwareClient<ClientRequest, HttpResponse> client,
|
||||
ZuulFallbackProvider fallbackProvider, int timeout) {
|
||||
// different name is used because of properties caching
|
||||
super(getSetter("testCommand" + UUID.randomUUID(), new ZuulProperties())
|
||||
super(getSetter("testCommand" + UUID.randomUUID(), new ZuulProperties(), new DefaultClientConfigImpl())
|
||||
.andCommandPropertiesDefaults(defauts(timeout)), client, null,
|
||||
fallbackProvider, null);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user