Merge pull request #21 from Netflix/bugfix/simpler_vegas

Misc cleanup
Netflix · Feb 13, 2018 · 72fe8b6 · 72fe8b6
2 parents 220468f + 905dc30
commit 72fe8b6
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 15 deletions.
diff --git a/concurrency-limits-core/src/main/java/com/netflix/concurrency/limits/limit/VegasLimit.java b/concurrency-limits-core/src/main/java/com/netflix/concurrency/limits/limit/VegasLimit.java
@@ -34,7 +34,7 @@ public static class Builder {
         private Function<Integer, Integer> alpha = (limit) -> 3;
         private Function<Integer, Integer> beta = (limit) -> 6;
         private Function<Integer, Integer> increaseFunc = (limit) -> limit + 1;
-        private Function<Integer, Integer> decreaseFunc = (limit) -> limit / 2;
+        private Function<Integer, Integer> decreaseFunc = (limit) -> limit - 1;
 
         public Builder alpha(int alpha) {
             this.alpha = (ignore) -> alpha;
@@ -86,6 +86,7 @@ public Builder maxConcurrency(int maxConcurrency) {
             return this;
         }
 
+        @Deprecated
         public Builder backoffRatio(double ratio) {
             return this;
         }
@@ -158,7 +159,7 @@ public synchronized void update(long rtt) {
             int alpha = alphaFunc.apply((int)estimatedLimit);
             int beta = betaFunc.apply((int)estimatedLimit);
 
-            if (queueSize <= alpha) {
+            if (queueSize < alpha) {
                 newLimit = increaseFunc.apply((int)estimatedLimit);
             } else if (queueSize > beta) {
                 newLimit = decreaseFunc.apply((int)estimatedLimit);
@@ -169,15 +170,12 @@ public synchronized void update(long rtt) {
 
         newLimit = Math.max(1, Math.min(maxLimit, newLimit));
         newLimit = (int) ((1 - smoothing) * estimatedLimit + smoothing * newLimit);
-        if ((int)newLimit != (int)estimatedLimit) {
-            estimatedLimit = newLimit;
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("New limit={} minRtt={} μs winRtt={} μs queueSize={}", 
-                        estimatedLimit, 
-                        TimeUnit.NANOSECONDS.toMicros(rtt_noload), 
-                        TimeUnit.NANOSECONDS.toMicros(rtt),
-                        queueSize);
-            }
+        if ((int)newLimit != (int)estimatedLimit && LOG.isDebugEnabled()) {
+            LOG.debug("New limit={} minRtt={} μs winRtt={} μs queueSize={}", 
+                    estimatedLimit, 
+                    TimeUnit.NANOSECONDS.toMicros(rtt_noload), 
+                    TimeUnit.NANOSECONDS.toMicros(rtt),
+                    queueSize);
         }
         estimatedLimit = newLimit;
     }

diff --git a/...rrency-limits-core/src/test/java/com/netflix/concurrency/limits/limit/VegasLimitTest.java b/...rrency-limits-core/src/test/java/com/netflix/concurrency/limits/limit/VegasLimitTest.java
@@ -38,7 +38,7 @@ public void decreaseLimit() {
         limit.update(TimeUnit.MILLISECONDS.toNanos(10));
         Assert.assertEquals(11, limit.getLimit());
         limit.update(TimeUnit.MILLISECONDS.toNanos(50));
-        Assert.assertEquals(5, limit.getLimit());
+        Assert.assertEquals(10, limit.getLimit());
     }
 
     @Test

diff --git a/...r/src/test/java/com/netflix/concurrency/limits/spectator/SpectatorMetricRegistryTest.java b/...r/src/test/java/com/netflix/concurrency/limits/spectator/SpectatorMetricRegistryTest.java
@@ -1,11 +1,11 @@
 package com.netflix.concurrency.limits.spectator;
 
-import com.netflix.spectator.api.DefaultRegistry;
-import com.netflix.spectator.api.patterns.PolledMeter;
-
 import org.junit.Assert;
 import org.junit.Test;
 
+import com.netflix.spectator.api.DefaultRegistry;
+import com.netflix.spectator.api.patterns.PolledMeter;
+
 public class SpectatorMetricRegistryTest {
     @Test
     public void testGuage() {
@@ -18,4 +18,18 @@ public void testGuage() {
 
         Assert.assertEquals(10.0, registry.gauge(registry.createId("foo.bar")).value(), 0);
     }
+
+    @Test
+    public void testUnregister() {
+        DefaultRegistry registry = new DefaultRegistry();
+        SpectatorMetricRegistry metricRegistry = new SpectatorMetricRegistry(registry, registry.createId("foo"));
+
+        metricRegistry.registerGauge("bar", () -> 10);
+        metricRegistry.registerGauge("bar", () -> 20);
+
+        PolledMeter.update(registry);
+
+        Assert.assertEquals(20.0, registry.gauge(registry.createId("foo.bar")).value(), 0);
+
+    }
 }