Mailing-List: contact commits-help@geode.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geode.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: dschneider@apache.org
To: commits@geode.incubator.apache.org
Date: Tue, 03 Nov 2015 19:38:36 -0000
Message-Id: <71b94596490c4664ba1e8888c25253bd@git.apache.org>
In-Reply-To: <de269e6fbfca4e35a1de6c35ce19719a@git.apache.org>
References: <de269e6fbfca4e35a1de6c35ce19719a@git.apache.org>
Subject: [04/35] incubator-geode git commit: GEODE-464: Fix AutoBalancer test
 race condition

GEODE-464: Fix AutoBalancer test race condition

Instance of cache Initializer is not destroyed when the cache is destroyed. As a
result, a scheduled rebalance job can start after a different test starts. This
can cause failures. Add a destroy method to kill scheduler in test tearDown to
prevent orphaned tasks from failing other tests.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/82966c46
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/82966c46
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/82966c46

Branch: refs/heads/develop
Commit: 82966c466a219f6a3674af0e702b9b12194c8c38
Parents: b6446dc
Author: Ashvin Agrawal <ashvin@apache.org>
Authored: Mon Oct 26 15:28:22 2015 -0700
Committer: Ashvin Agrawal <ashvin@apache.org>
Committed: Mon Oct 26 15:36:42 2015 -0700

----------------------------------------------------------------------
 .../gemfire/cache/util/AutoBalancer.java        | 14 +++++++
 .../util/AutoBalancerIntegrationJUnitTest.java  |  5 +++
 .../cache/util/AutoBalancerJUnitTest.java       | 39 ++++++++++++++++++++
 3 files changed, 58 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/82966c46/gemfire-rebalancer/src/main/java/com/gemstone/gemfire/cache/util/AutoBalancer.java
----------------------------------------------------------------------
diff --git a/gemfire-rebalancer/src/main/java/com/gemstone/gemfire/cache/util/AutoBalancer.java b/gemfire-rebalancer/src/main/java/com/gemstone/gemfire/cache/util/AutoBalancer.java
index 633ae39..48fec09 100644
--- a/gemfire-rebalancer/src/main/java/com/gemstone/gemfire/cache/util/AutoBalancer.java
+++ b/gemfire-rebalancer/src/main/java/com/gemstone/gemfire/cache/util/AutoBalancer.java
@@ -225,6 +225,9 @@ public class AutoBalancer implements Declarable {
         public void run() {
           try {
             auditor.execute();
+          } catch (CacheClosedException e) {
+            logger.warn("Cache closed while attempting to rebalance the cluster. Abort future jobs", e);
+            return;
           } catch (Exception e) {
             logger.warn("Error while executing out-of-balance audit.", e);
           }
@@ -232,6 +235,11 @@ public class AutoBalancer implements Declarable {
         }
       }, delay, TimeUnit.MILLISECONDS);
     }
+
+    @Override
+    public void destroy() {
+      trigger.shutdownNow();
+    }
   }
 
   /**
@@ -502,6 +510,8 @@ public class AutoBalancer implements Declarable {
 
   interface AuditScheduler {
     void init(String schedule);
+
+    void destroy();
   }
 
   interface OOBAuditor {
@@ -537,4 +547,8 @@ public class AutoBalancer implements Declarable {
   public CacheOperationFacade getCacheOperationFacade() {
     return this.cacheFacade;
   }
+
+  public void destroy() {
+    scheduler.destroy();
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/82966c46/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerIntegrationJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerIntegrationJUnitTest.java b/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerIntegrationJUnitTest.java
index cff9d69..e50103c 100755
--- a/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerIntegrationJUnitTest.java
+++ b/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerIntegrationJUnitTest.java
@@ -51,6 +51,11 @@ public class AutoBalancerIntegrationJUnitTest {
       DLockService.destroy(AutoBalancer.AUTO_BALANCER_LOCK_SERVICE_NAME);
     }
 
+    AutoBalancer autoBalancer = (AutoBalancer) cache.getInitializer();
+    if (autoBalancer != null) {
+      autoBalancer.destroy();
+    }
+
     if (cache != null && !cache.isClosed()) {
       try {
         final HostStatSampler statSampler = ((InternalDistributedSystem) cache.getDistributedSystem()).getStatSampler();

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/82966c46/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerJUnitTest.java b/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerJUnitTest.java
index c0b6725..cb88022 100644
--- a/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerJUnitTest.java
+++ b/gemfire-rebalancer/src/test/java/com/gemstone/gemfire/cache/util/AutoBalancerJUnitTest.java
@@ -556,6 +556,45 @@ public class AutoBalancerJUnitTest {
     assertTrue(latch.await(1, TimeUnit.SECONDS));
   }
 
+  @Test
+  public void destroyAutoBalancer() throws InterruptedException {
+    final CountDownLatch latch = new CountDownLatch(2);
+    final CountDownLatch timerLatch = new CountDownLatch(1);
+    final int timer = 20; // simulate 20 milliseconds
+
+    mockContext.checking(new Expectations() {
+      {
+        oneOf(mockAuditor).init(with(any(Properties.class)));
+        allowing(mockAuditor).execute();
+        allowing(mockClock).currentTimeMillis();
+        will(new CustomAction("returnTime") {
+          @Override
+          public Object invoke(Invocation invocation) throws Throwable {
+            latch.countDown();
+            if (latch.getCount() == 0) {
+              assertTrue(timerLatch.await(1, TimeUnit.SECONDS));
+              // scheduler is destroyed before wait is over
+              fail();
+            }
+            return 1000L - timer;
+          }
+        });
+      }
+    });
+
+    Properties props = AutoBalancerJUnitTest.getBasicConfig();
+
+    assertEquals(2, latch.getCount());
+    AutoBalancer autoR = new AutoBalancer(null, mockAuditor, mockClock, null);
+    autoR.init(props);
+    assertTrue(latch.await(1, TimeUnit.SECONDS));
+
+    // after destroy no more execute will be called.
+    autoR.destroy();
+    timerLatch.countDown();
+    TimeUnit.MILLISECONDS.sleep(2 * timer);
+  }
+
   static Properties getBasicConfig() {
     Properties props = new Properties();
     // every second schedule