Add options for RibbonCommand to use separate thread pools for hystrix (#2074)

docs/src/main/asciidoc/spring-cloud-netflix.adoc

@@ -886,6 +886,28 @@ ribbon:
     clients: client1, client2, client3
 ----
 
+[[how-to-configure-hystrix-thread-pools]]
+=== How to Configure Hystrix thread pools
+If you change `zuul.ribbonIsolationStrategy` to THREAD, the thread isolation strategy for Hystrix will be used for all routes. In this case, the HystrixThreadPoolKey is set to "RibbonCommand" as default. It means that HystrixCommands for all routes will be executed in the same Hystrix thread pool. This behavior can be changed using the following configuration and it will result in HystrixCommands being executed in the Hystrix thread pool for each route.
+
+.application.yml
+----
+zuul:
+  threadPool:
+    useSeparateThreadPools: true
+----
+
+The default HystrixThreadPoolKey in this case is same with service ID for each route. To add a prefix to HystrixThreadPoolKey, set `zuul.threadPool.threadPoolKeyPrefix` to a value that you want to add. For example:
+
+.application.yml
+----
+zuul:
+  threadPool:
+    useSeparateThreadPools: true
+    threadPoolKeyPrefix: zuulgw
+----
+
+
 
 [[spring-cloud-feign]]
 == Declarative REST Client: Feign

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java

@@ -160,6 +160,8 @@ public class ZuulProperties {
 	private ExecutionIsolationStrategy ribbonIsolationStrategy = SEMAPHORE;
 	
 	private HystrixSemaphore semaphore = new HystrixSemaphore();
+
+	private HystrixThreadPool threadPool = new HystrixThreadPool();
 	
 	public Set<String> getIgnoredHeaders() {
 		Set<String> ignoredHeaders = new LinkedHashSet<>(this.ignoredHeaders);
@@ -357,6 +359,39 @@ public class ZuulProperties {
 		
 	}
 
+	public static class HystrixThreadPool {
+		/**
+		 * Flag to determine whether RibbonCommands should use separate thread pools for hystrix.
+		 * By setting to true, RibbonCommands will be executed in a hystrix's thread pool that it is associated with.
+		 * Each RibbonCommand will be associated with a thread pool according to its commandKey (serviceId).
+		 * As default, all commands will be executed in a single thread pool whose threadPoolKey is "RibbonCommand".
+		 * This property is only applicable when using THREAD as ribbonIsolationStrategy
+		 */
+		private boolean useSeparateThreadPools = false;
+
+		/**
+		 * A prefix for HystrixThreadPoolKey of hystrix's thread pool that is allocated to each service Id.
+		 * This property is only applicable when using THREAD as ribbonIsolationStrategy and useSeparateThreadPools = true
+		 */
+		private String threadPoolKeyPrefix = "";
+
+		public boolean isUseSeparateThreadPools() {
+			return useSeparateThreadPools;
+		}
+
+		public void setUseSeparateThreadPools(boolean useSeparateThreadPools) {
+			this.useSeparateThreadPools = useSeparateThreadPools;
+		}
+
+		public String getThreadPoolKeyPrefix() {
+			return threadPoolKeyPrefix;
+		}
+
+		public void setThreadPoolKeyPrefix(String threadPoolKeyPrefix) {
+			this.threadPoolKeyPrefix = threadPoolKeyPrefix;
+		}
+	}
+
 	public String getServletPattern() {
 		String path = this.servletPath;
 		if (!path.startsWith("/")) {

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java

@@ -34,6 +34,7 @@ import com.netflix.hystrix.HystrixCommandGroupKey;
 import com.netflix.hystrix.HystrixCommandKey;
 import com.netflix.hystrix.HystrixCommandProperties;
 import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy;
+import com.netflix.hystrix.HystrixThreadPoolKey;
 import com.netflix.zuul.constants.ZuulConstants;
 import com.netflix.zuul.context.RequestContext;
 
@@ -78,6 +79,9 @@ public abstract class AbstractRibbonCommand<LBC extends AbstractLoadBalancerAwar
 			ZuulProperties zuulProperties) {
 
 		// @formatter:off
+		Setter commandSetter = Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("RibbonCommand"))
+								.andCommandKey(HystrixCommandKey.Factory.asKey(commandKey));
+
 		final HystrixCommandProperties.Setter setter = HystrixCommandProperties.Setter()
 				.withExecutionIsolationStrategy(zuulProperties.getRibbonIsolationStrategy());
 		if (zuulProperties.getRibbonIsolationStrategy() == ExecutionIsolationStrategy.SEMAPHORE){
@@ -87,13 +91,12 @@ public abstract class AbstractRibbonCommand<LBC extends AbstractLoadBalancerAwar
 			final DynamicIntProperty value = DynamicPropertyFactory.getInstance()
 					.getIntProperty(name, zuulProperties.getSemaphore().getMaxSemaphores());
 			setter.withExecutionIsolationSemaphoreMaxConcurrentRequests(value.get());
-		} else	{
-			// TODO Find out is some parameters can be set here
+		} else if (zuulProperties.getThreadPool().isUseSeparateThreadPools()) {
+			final String threadPoolKey = zuulProperties.getThreadPool().getThreadPoolKeyPrefix() + commandKey;
+			commandSetter.andThreadPoolKey(HystrixThreadPoolKey.Factory.asKey(threadPoolKey));
 		}
 		
-		return Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("RibbonCommand"))
-				.andCommandKey(HystrixCommandKey.Factory.asKey(commandKey))
-				.andCommandPropertiesDefaults(setter);
+		return commandSetter.andCommandPropertiesDefaults(setter);
 		// @formatter:on
 	}
 

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java

@@ -24,6 +24,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -103,4 +104,9 @@ public class ZuulPropertiesTests {
 		assertFalse(this.zuul.getSensitiveHeaders().contains("Cookie"));
 	}
 
+	@Test
+	public void defaultHystrixThreadPool() {
+		assertFalse(this.zuul.getThreadPool().isUseSeparateThreadPools());
+		assertEquals("", this.zuul.getThreadPool().getThreadPoolKeyPrefix());
+	}
 }

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandHystrixThreadPoolKeyTests.java

+/*
+ * Copyright 2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.netflix.zuul.filters.route.support;
+
+import com.netflix.client.ClientRequest;
+import com.netflix.hystrix.HystrixCommandProperties;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * @author Yongsung Yoon
+ */
+public class RibbonCommandHystrixThreadPoolKeyTests {
+
+	private ZuulProperties zuulProperties;
+
+	@Before
+	public void setUp() throws Exception {
+		zuulProperties = new ZuulProperties();
+	}
+
+	@Test
+	public void testDefaultHystrixThreadPoolKey() throws Exception {
+		zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD);
+
+		TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties);
+		TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties);
+
+		// CommandGroupKey should be used as ThreadPoolKey as default.
+		assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(ribbonCommand1.getCommandGroup().name());
+		assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(ribbonCommand2.getCommandGroup().name());
+	}
+
+	@Test
+	public void testUseSeparateThreadPools() throws Exception {
+		zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD);
+		zuulProperties.getThreadPool().setUseSeparateThreadPools(true);
+
+		TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties);
+		TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties);
+
+		assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo("testCommand1");
+		assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo("testCommand2");
+	}
+
+	@Test
+	public void testThreadPoolKeyPrefix() throws Exception {
+		final String prefix = "zuulgw-";
+
+		zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD);
+		zuulProperties.getThreadPool().setUseSeparateThreadPools(true);
+		zuulProperties.getThreadPool().setThreadPoolKeyPrefix(prefix);
+
+		TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties);
+		TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties);
+
+		assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(prefix + "testCommand1");
+		assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(prefix + "testCommand2");
+	}
+
+	@Test
+	public void testNoSideEffectOnSemaphoreIsolation() throws Exception {
+		final String prefix = "zuulgw-";
+
+		zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE);
+		zuulProperties.getThreadPool().setUseSeparateThreadPools(true);
+		zuulProperties.getThreadPool().setThreadPoolKeyPrefix(prefix);
+
+		TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties);
+		TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties);
+
+		// There should be no side effect on semaphore isolation
+		assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(ribbonCommand1.getCommandGroup().name());
+		assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(ribbonCommand2.getCommandGroup().name());
+	}
+
+	public static class TestRibbonCommand extends AbstractRibbonCommand {
+		public TestRibbonCommand(String commandKey, ZuulProperties zuulProperties) {
+			super(commandKey, null, null, zuulProperties);
+		}
+
+		@Override
+		protected ClientRequest createRequest() throws Exception {
+			return null;
+		}
+	}
+}
\ No newline at end of file