Merge pull request #1449 from mattrjacobs/single-method-which-ensures-core-less-than-equal-to-maximum

mattrjacobs · web-flow · commit 3c6b1b03c535 · 2016-12-20T20:40:51.000-08:00
Single method which ensures core less than equal to maximum
diff --git a/hystrix-contrib/hystrix-codahale-metrics-publisher/src/main/java/com/netflix/hystrix/contrib/codahalemetricspublisher/HystrixCodaHaleMetricsPublisherThreadPool.java b/hystrix-contrib/hystrix-codahale-metrics-publisher/src/main/java/com/netflix/hystrix/contrib/codahalemetricspublisher/HystrixCodaHaleMetricsPublisherThreadPool.java
@@ -146,6 +146,20 @@ public Number getValue() {
             }
         });
 
+        metricRegistry.register(createMetricName("propertyValue_maximumSize"), new Gauge<Number>() {
+            @Override
+            public Number getValue() {
+                return properties.maximumSize().get();
+            }
+        });
+
+        metricRegistry.register(createMetricName("propertyValue_actualMaximumSize"), new Gauge<Number>() {
+            @Override
+            public Number getValue() {
+                return properties.maximumSize().get();
+            }
+        });
+
         metricRegistry.register(createMetricName("propertyValue_keepAliveTimeInMinutes"), new Gauge<Number>() {
             @Override
             public Number getValue() {
diff --git a/hystrix-contrib/hystrix-metrics-event-stream/src/main/java/com/netflix/hystrix/contrib/sample/stream/HystrixConfigurationJsonStream.java b/hystrix-contrib/hystrix-metrics-event-stream/src/main/java/com/netflix/hystrix/contrib/sample/stream/HystrixConfigurationJsonStream.java
@@ -117,6 +117,7 @@ private static void writeThreadPoolConfigJson(JsonGenerator json, HystrixThreadP
         json.writeObjectFieldStart(threadPoolKey.name());
         json.writeNumberField("coreSize", threadPoolConfig.getCoreSize());
         json.writeNumberField("maximumSize", threadPoolConfig.getMaximumSize());
+        json.writeNumberField("actualMaximumSize", threadPoolConfig.getActualMaximumSize());
         json.writeNumberField("maxQueueSize", threadPoolConfig.getMaxQueueSize());
         json.writeNumberField("queueRejectionThreshold", threadPoolConfig.getQueueRejectionThreshold());
         json.writeNumberField("keepAliveTimeInMinutes", threadPoolConfig.getKeepAliveTimeInMinutes());
diff --git a/hystrix-contrib/hystrix-servo-metrics-publisher/src/main/java/com/netflix/hystrix/contrib/servopublisher/HystrixServoMetricsPublisherThreadPool.java b/hystrix-contrib/hystrix-servo-metrics-publisher/src/main/java/com/netflix/hystrix/contrib/servopublisher/HystrixServoMetricsPublisherThreadPool.java
@@ -249,6 +249,20 @@ public Number getValue() {
             }
         });
 
+        monitors.add(new InformationalMetric<Number>(MonitorConfig.builder("propertyValue_maximumSize").build()) {
+            @Override
+            public Number getValue() {
+                return properties.maximumSize().get();
+            }
+        });
+
+        monitors.add(new InformationalMetric<Number>(MonitorConfig.builder("propertyValue_actualMaximumSize").build()) {
+            @Override
+            public Number getValue() {
+                return properties.actualMaximumSize();
+            }
+        });
+
         monitors.add(new InformationalMetric<Number>(MonitorConfig.builder("propertyValue_keepAliveTimeInMinutes").build()) {
             @Override
             public Number getValue() {
diff --git a/hystrix-contrib/hystrix-yammer-metrics-publisher/src/main/java/com/netflix/hystrix/contrib/yammermetricspublisher/HystrixYammerMetricsPublisherThreadPool.java b/hystrix-contrib/hystrix-yammer-metrics-publisher/src/main/java/com/netflix/hystrix/contrib/yammermetricspublisher/HystrixYammerMetricsPublisherThreadPool.java
@@ -142,6 +142,20 @@ public Number value() {
             }
         });
 
+        metricsRegistry.newGauge(createMetricName("propertyValue_maximumSize"), new Gauge<Number>() {
+            @Override
+            public Number value() {
+                return properties.maximumSize().get();
+            }
+        });
+
+        metricsRegistry.newGauge(createMetricName("propertyValue_actualMaximumSize"), new Gauge<Number>() {
+            @Override
+            public Number value() {
+                return properties.actualMaximumSize();
+            }
+        });
+
         metricsRegistry.newGauge(createMetricName("propertyValue_keepAliveTimeInMinutes"), new Gauge<Number>() {
             @Override
             public Number value() {
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixThreadPool.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixThreadPool.java
@@ -209,31 +209,26 @@ public Scheduler getScheduler(Func0<Boolean> shouldInterruptThread) {
         // allow us to change things via fast-properties by setting it each time
         private void touchConfig() {
             final int dynamicCoreSize = properties.coreSize().get();
-            final int dynamicMaximumSize = properties.maximumSize().get();
-            int updatedMaximumSize = dynamicMaximumSize;
+            final int configuredMaximumSize = properties.maximumSize().get();
+            int dynamicMaximumSize = properties.actualMaximumSize();
             final boolean allowSizesToDiverge = properties.getAllowMaximumSizeToDivergeFromCoreSize().get();
             boolean maxTooLow = false;
 
-            if (allowSizesToDiverge && dynamicMaximumSize < dynamicCoreSize) {
+            if (allowSizesToDiverge && configuredMaximumSize < dynamicCoreSize) {
                 //if user sets maximum < core (or defaults get us there), we need to maintain invariant of core <= maximum
-                updatedMaximumSize = dynamicCoreSize;
+                dynamicMaximumSize = dynamicCoreSize;
                 maxTooLow = true;
             }
 
-            if (!allowSizesToDiverge) {
-                //if user has not opted in to allowing sizes to diverge, ensure maximum == core
-                updatedMaximumSize = dynamicCoreSize;
-            }
-
             // In JDK 6, setCorePoolSize and setMaximumPoolSize will execute a lock operation. Avoid them if the pool size is not changed.
-            if (threadPool.getCorePoolSize() != dynamicCoreSize || (allowSizesToDiverge && threadPool.getMaximumPoolSize() != updatedMaximumSize)) {
+            if (threadPool.getCorePoolSize() != dynamicCoreSize || (allowSizesToDiverge && threadPool.getMaximumPoolSize() != dynamicMaximumSize)) {
                 if (maxTooLow) {
                     logger.error("Hystrix ThreadPool configuration for : " + metrics.getThreadPoolKey().name() + " is trying to set coreSize = " +
-                            dynamicCoreSize + " and maximumSize = " + dynamicMaximumSize + ".  Maximum size will be set to " +
-                            dynamicCoreSize + ", the coreSize value, since it must be equal to or greater than the coreSize value");
+                            dynamicCoreSize + " and maximumSize = " + configuredMaximumSize + ".  Maximum size will be set to " +
+                            dynamicMaximumSize + ", the coreSize value, since it must be equal to or greater than the coreSize value");
                 }
                 threadPool.setCorePoolSize(dynamicCoreSize);
-                threadPool.setMaximumPoolSize(updatedMaximumSize);
+                threadPool.setMaximumPoolSize(dynamicMaximumSize);
             }
 
             threadPool.setKeepAliveTime(properties.keepAliveTimeMinutes().get(), TimeUnit.MINUTES);
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixThreadPoolProperties.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixThreadPoolProperties.java
@@ -116,14 +116,39 @@ public HystrixProperty<Integer> coreSize() {
     }
 
     /**
-     * Maximum thread-pool size that gets passed to {@link ThreadPoolExecutor#setMaximumPoolSize(int)}
+     * Maximum thread-pool size configured for threadpool.  May conflict with other config, so if you need the
+     * actual value that gets passed to {@link ThreadPoolExecutor#setMaximumPoolSize(int)}, use {@link #actualMaximumSize()}
      *
      * @return {@code HystrixProperty<Integer>}
      */
     public HystrixProperty<Integer> maximumSize() {
         return maximumPoolSize;
     }
 
+    /**
+     * Given all of the thread pool configuration, what is the actual maximumSize applied to the thread pool
+     * via {@link ThreadPoolExecutor#setMaximumPoolSize(int)}
+     *
+     * Cases:
+     * 1) allowMaximumSizeToDivergeFromCoreSize == false: maximumSize is set to coreSize
+     * 2) allowMaximumSizeToDivergeFromCoreSize == true, maximumSize >= coreSize: thread pool has different core/max sizes, so return the configured max
+     * 3) allowMaximumSizeToDivergeFromCoreSize == true, maximumSize < coreSize: threadpool incorrectly configured, use coreSize for max size
+     * @return actually configured maximum size of threadpool
+     */
+    public Integer actualMaximumSize() {
+        final int coreSize = coreSize().get();
+        final int maximumSize = maximumSize().get();
+        if (getAllowMaximumSizeToDivergeFromCoreSize().get()) {
+            if (coreSize > maximumSize) {
+                return coreSize;
+            } else {
+                return maximumSize;
+            }
+        } else {
+            return coreSize;
+        }
+    }
+
     /**
      * Keep-alive time in minutes that gets passed to {@link ThreadPoolExecutor#setKeepAliveTime(long, TimeUnit)}
      * 
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/config/HystrixThreadPoolConfiguration.java b/hystrix-core/src/main/java/com/netflix/hystrix/config/HystrixThreadPoolConfiguration.java
@@ -19,53 +19,45 @@
 import com.netflix.hystrix.HystrixThreadPoolProperties;
 
 public class HystrixThreadPoolConfiguration {
-    //The idea is for this object to be serialized off-box.  For future-proofing, I'm adding a version so that changing config over time can be handled gracefully
-    private static final String VERSION = "1";
     private final HystrixThreadPoolKey threadPoolKey;
     private final int coreSize;
     private final int maximumSize;
+    private final int actualMaximumSize;
     private final int maxQueueSize;
     private final int queueRejectionThreshold;
     private final int keepAliveTimeInMinutes;
     private final boolean allowMaximumSizeToDivergeFromCoreSize;
     private final int rollingCounterNumberOfBuckets;
     private final int rollingCounterBucketSizeInMilliseconds;
 
-    public HystrixThreadPoolConfiguration(HystrixThreadPoolKey threadPoolKey, int coreSize, int maximumSize, int maxQueueSize, int queueRejectionThreshold,
+    private HystrixThreadPoolConfiguration(HystrixThreadPoolKey threadPoolKey, int coreSize, int maximumSize, int actualMaximumSize, int maxQueueSize, int queueRejectionThreshold,
                                            int keepAliveTimeInMinutes, boolean allowMaximumSizeToDivergeFromCoreSize, int rollingCounterNumberOfBuckets,
                                            int rollingCounterBucketSizeInMilliseconds) {
         this.threadPoolKey = threadPoolKey;
         this.allowMaximumSizeToDivergeFromCoreSize = allowMaximumSizeToDivergeFromCoreSize;
         this.coreSize = coreSize;
-        if (allowMaximumSizeToDivergeFromCoreSize) {
-            if (coreSize > maximumSize) {
-                this.maximumSize = coreSize;
-            } else {
-                this.maximumSize = maximumSize;
-            }
-        } else {
-            this.maximumSize = coreSize;
-        }
+        this.maximumSize = maximumSize;
+        this.actualMaximumSize = actualMaximumSize;
         this.maxQueueSize = maxQueueSize;
         this.queueRejectionThreshold = queueRejectionThreshold;
         this.keepAliveTimeInMinutes = keepAliveTimeInMinutes;
         this.rollingCounterNumberOfBuckets = rollingCounterNumberOfBuckets;
         this.rollingCounterBucketSizeInMilliseconds = rollingCounterBucketSizeInMilliseconds;
     }
 
-    public static HystrixThreadPoolConfiguration sample(HystrixThreadPoolKey threadPoolKey, HystrixThreadPoolProperties threadPoolProperties) {
-        return new HystrixThreadPoolConfiguration(
-                threadPoolKey,
-                threadPoolProperties.coreSize().get(),
-                threadPoolProperties.maximumSize().get(),
-                threadPoolProperties.maxQueueSize().get(),
-                threadPoolProperties.queueSizeRejectionThreshold().get(),
-                threadPoolProperties.keepAliveTimeMinutes().get(),
-                threadPoolProperties.getAllowMaximumSizeToDivergeFromCoreSize().get(),
+    private HystrixThreadPoolConfiguration(HystrixThreadPoolKey threadPoolKey, HystrixThreadPoolProperties threadPoolProperties) {
+        this(threadPoolKey, threadPoolProperties.coreSize().get(),
+                threadPoolProperties.maximumSize().get(), threadPoolProperties.actualMaximumSize(),
+                threadPoolProperties.maxQueueSize().get(), threadPoolProperties.queueSizeRejectionThreshold().get(),
+                threadPoolProperties.keepAliveTimeMinutes().get(), threadPoolProperties.getAllowMaximumSizeToDivergeFromCoreSize().get(),
                 threadPoolProperties.metricsRollingStatisticalWindowBuckets().get(),
                 threadPoolProperties.metricsRollingStatisticalWindowInMilliseconds().get());
     }
 
+    public static HystrixThreadPoolConfiguration sample(HystrixThreadPoolKey threadPoolKey, HystrixThreadPoolProperties threadPoolProperties) {
+        return new HystrixThreadPoolConfiguration(threadPoolKey, threadPoolProperties);
+    }
+
     public HystrixThreadPoolKey getThreadPoolKey() {
         return threadPoolKey;
     }
@@ -74,12 +66,25 @@ public int getCoreSize() {
         return coreSize;
     }
 
+    /**
+     * Simple getter that returns what the `maximumSize` property is configured as
+     * @return
+     */
     public int getMaximumSize() {
-        if (allowMaximumSizeToDivergeFromCoreSize) {
-            return maximumSize;
-        } else {
-            return coreSize;
-        }
+        return maximumSize;
+    }
+
+    /**
+     * Given all of the thread pool configuration, what is the actual maximumSize applied to the thread pool.
+     *
+     * Cases:
+     * 1) allowMaximumSizeToDivergeFromCoreSize == false: maximumSize is set to coreSize
+     * 2) allowMaximumSizeToDivergeFromCoreSize == true, maximumSize >= coreSize: thread pool has different core/max sizes, so return the configured max
+     * 3) allowMaximumSizeToDivergeFromCoreSize == true, maximumSize < coreSize: threadpool incorrectly configured, use coreSize for max size
+     * @return actually configured maximum size of threadpool
+     */
+    public int getActualMaximumSize() {
+        return this.actualMaximumSize;
     }
 
     public int getMaxQueueSize() {
diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixThreadPoolPropertiesTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixThreadPoolPropertiesTest.java
diff --git a/hystrix-serialization/src/main/java/com/netflix/hystrix/serial/SerialHystrixConfiguration.java b/hystrix-serialization/src/main/java/com/netflix/hystrix/serial/SerialHystrixConfiguration.java
