From 1f0db2ad22ec6f7ee7d09e09ac5be3d8a1bf6a64 Mon Sep 17 00:00:00 2001
From: Sarge <mdodge@pivotal.io>
Date: Fri, 5 Jan 2018 10:50:56 -0800
Subject: [PATCH] GEODE-4147: Add variability to client rebalance logic

---
 .../geode/cache/client/internal/PoolImpl.java | 23 ++++++++++++++++++-
 .../internal/ConnectionPoolImplJUnitTest.java | 13 ++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java
index c266b8bb3aa8..897029fb52fd 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java
@@ -25,6 +25,7 @@
 import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.Set;
+import java.util.SplittableRandom;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ThreadFactory;
@@ -159,6 +160,26 @@ public static PoolImpl create(PoolManagerImpl pm, String name, Pool attributes,
     return pool;
   }
 
+  /**
+   * Adds an arbitrary variance to a positive temporal interval. Where possible, 10% of the interval
+   * is added or subtracted from the interval. Otherwise, 1 is added or subtracted from the
+   * interval. For all positive intervals, the returned value will <bold>not</bold> equal the
+   * supplied interval.
+   *
+   * @param interval Positive temporal interval.
+   * @return Adjusted interval including the variance for positive intervals; the unmodified
+   *         interval for non-positive intervals.
+   */
+  static int addVarianceToInterval(int interval) {
+    if (1 <= interval) {
+      final SplittableRandom random = new SplittableRandom();
+      final int variance = (interval < 10) ? 1 : random.nextInt(interval / 10);
+      final int sign = random.nextBoolean() ? 1 : -1;
+      return interval + (sign * variance);
+    }
+    return interval;
+  }
+
   public boolean isUsedByGateway() {
     return usedByGateway;
   }
@@ -186,7 +207,7 @@ protected PoolImpl(PoolManagerImpl pm, String name, Pool attributes,
     this.name = name;
     this.socketConnectTimeout = attributes.getSocketConnectTimeout();
     this.freeConnectionTimeout = attributes.getFreeConnectionTimeout();
-    this.loadConditioningInterval = attributes.getLoadConditioningInterval();
+    this.loadConditioningInterval = addVarianceToInterval(attributes.getLoadConditioningInterval());
     this.socketBufferSize = attributes.getSocketBufferSize();
     this.threadLocalConnections = attributes.getThreadLocalConnections();
     this.readTimeout = attributes.getReadTimeout();
diff --git a/geode-core/src/test/java/org/apache/geode/cache/client/internal/ConnectionPoolImplJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/client/internal/ConnectionPoolImplJUnitTest.java
index da87243fc1cf..c227320a951d 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/client/internal/ConnectionPoolImplJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/client/internal/ConnectionPoolImplJUnitTest.java
@@ -16,7 +16,10 @@
 
 import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
 import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
-import static org.junit.Assert.*;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.net.InetSocketAddress;
 import java.net.SocketTimeoutException;
@@ -267,4 +270,12 @@ public boolean useThreadLocalConnection() {
     assertEquals(location1, pool.executeOnQueuesAndReturnPrimaryResult(testOp));
   }
 
+  @Test
+  public void testAddVarianceToInterval() {
+    assertThat(PoolImpl.addVarianceToInterval(0)).as("Zero gets zero variance").isEqualTo(0);
+    assertThat(PoolImpl.addVarianceToInterval(300000)).as("Large value gets +/-10% variance")
+        .isNotEqualTo(300000).isGreaterThanOrEqualTo(270000).isLessThanOrEqualTo(330000);
+    assertThat(PoolImpl.addVarianceToInterval(9)).as("Small value gets +/-1 variance")
+        .isNotEqualTo(9).isGreaterThanOrEqualTo(8).isLessThanOrEqualTo(10);
+  }
 }