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 <[email protected]> Authored: Mon Oct 26 15:28:22 2015 -0700 Committer: Ashvin Agrawal <[email protected]> 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
