Repository: geode Updated Branches: refs/heads/feature/GEM-1299 788eed117 -> 435ab7c1a (forced update)
GEODE-576 & GEODE-516: GemFireDeadlockDetectorDUnitTest failures Replaced pauses with Awaitility. Modified asyncs to use the DUnit blackboard to synchronize their actions for repeatable behavior. Cleaned up static locks to allow their reuse in other tests or in repeating the same test. Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/50686b0b Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/50686b0b Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/50686b0b Branch: refs/heads/feature/GEM-1299 Commit: 50686b0b44024d2bcbb4bea8a36ce3a40ac158c2 Parents: 5891ed7 Author: Bruce Schuchardt <bschucha...@pivotal.io> Authored: Fri Apr 21 09:05:07 2017 -0700 Committer: Bruce Schuchardt <bschucha...@pivotal.io> Committed: Fri Apr 21 09:06:14 2017 -0700 ---------------------------------------------------------------------- .../GemFireDeadlockDetectorDUnitTest.java | 116 +++++++++++-------- .../geode/test/dunit/DUnitBlackboard.java | 13 +++ .../test/dunit/internal/InternalBlackboard.java | 5 + .../dunit/internal/InternalBlackboardImpl.java | 5 + 4 files changed, 91 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/50686b0b/geode-core/src/test/java/org/apache/geode/distributed/internal/deadlock/GemFireDeadlockDetectorDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/deadlock/GemFireDeadlockDetectorDUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/deadlock/GemFireDeadlockDetectorDUnitTest.java index e0bbde0..4a03c2d 100644 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/deadlock/GemFireDeadlockDetectorDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/deadlock/GemFireDeadlockDetectorDUnitTest.java @@ -14,34 +14,14 @@ */ package org.apache.geode.distributed.internal.deadlock; -import org.apache.geode.test.dunit.ThreadUtils; -import org.junit.experimental.categories.Category; -import org.junit.Test; - -import static org.junit.Assert.*; - -import org.awaitility.Awaitility; -import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase; -import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase; -import org.apache.geode.test.junit.categories.DistributedTest; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; -import java.util.Collections; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.Set; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; - -import org.junit.experimental.categories.Category; - -import org.apache.geode.cache.CacheFactory; import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.FunctionContext; import org.apache.geode.cache.execute.FunctionService; import org.apache.geode.cache.execute.ResultCollector; -import org.apache.geode.cache30.CacheTestCase; import org.apache.geode.distributed.DistributedLockService; import org.apache.geode.distributed.DistributedSystemDisconnectedException; import org.apache.geode.distributed.LockServiceDestroyedException; @@ -54,7 +34,20 @@ import org.apache.geode.test.dunit.LogWriterUtils; import org.apache.geode.test.dunit.SerializableCallable; import org.apache.geode.test.dunit.SerializableRunnable; import org.apache.geode.test.dunit.VM; -import org.apache.geode.test.junit.categories.FlakyTest; +import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase; +import org.apache.geode.test.junit.categories.DistributedTest; +import org.awaitility.Awaitility; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; @Category(DistributedTest.class) public class GemFireDeadlockDetectorDUnitTest extends JUnit4CacheTestCase { @@ -108,7 +101,8 @@ public class GemFireDeadlockDetectorDUnitTest extends JUnit4CacheTestCase { private static final Lock lock = new ReentrantLock(); - @Category(FlakyTest.class) // GEODE-516 & GEODE-576: async actions, thread sleeps, time sensitive + // @Category(FlakyTest.class) // GEODE-516 & GEODE-576: async actions, thread sleeps, time + // sensitive @Test public void testDistributedDeadlockWithFunction() throws Throwable { Host host = Host.getHost(0); @@ -117,41 +111,62 @@ public class GemFireDeadlockDetectorDUnitTest extends JUnit4CacheTestCase { getSystem(); InternalDistributedMember member1 = createCache(vm0); final InternalDistributedMember member2 = createCache(vm1); + getBlackboard().initBlackboard(); // Have two threads lock locks on different members in different orders. + String gateOnMember1 = "gateOnMember1"; + String gateOnMember2 = "gateOnMember2"; // This thread locks the lock member1 first, then member2. - AsyncInvocation async1 = lockTheLocks(vm0, member2); - // This thread locks the lock member2 first, then member1. - AsyncInvocation async2 = lockTheLocks(vm1, member1); + AsyncInvocation async1 = lockTheLocks(vm0, member2, gateOnMember1, gateOnMember2); - Thread.sleep(5000); - GemFireDeadlockDetector detect = new GemFireDeadlockDetector(); - LinkedList<Dependency> deadlock = detect.find().findCycle(); - LogWriterUtils.getLogWriter().info("Deadlock=" + DeadlockDetector.prettyFormat(deadlock)); - assertEquals(8, deadlock.size()); - stopStuckThreads(); - async1.getResult(30000); - async2.getResult(30000); + // This thread locks the lock member2 first, then member1. + AsyncInvocation async2 = lockTheLocks(vm1, member1, gateOnMember2, gateOnMember1); + try { + final LinkedList<Dependency> deadlockHolder[] = new LinkedList[1]; + Awaitility.await("waiting for deadlock").atMost(20, TimeUnit.SECONDS).until(() -> { + GemFireDeadlockDetector detect = new GemFireDeadlockDetector(); + LinkedList<Dependency> deadlock = detect.find().findCycle(); + if (deadlock != null) { + deadlockHolder[0] = deadlock; + } + return deadlock != null; + }); + LinkedList<Dependency> deadlock = deadlockHolder[0]; + LogWriterUtils.getLogWriter().info("Deadlock=" + DeadlockDetector.prettyFormat(deadlock)); + assertEquals(8, deadlock.size()); + stopStuckThreads(); + } finally { + try { + waitForAsyncInvocation(async1, 45, TimeUnit.SECONDS); + } finally { + waitForAsyncInvocation(async2, 45, TimeUnit.SECONDS); + } + } } - private AsyncInvocation lockTheLocks(VM vm0, final InternalDistributedMember member) { + private AsyncInvocation lockTheLocks(VM vm0, final InternalDistributedMember member, + final String gateToSignal, final String gateToWaitOn) { return vm0.invokeAsync(new SerializableRunnable() { public void run() { lock.lock(); try { - Thread.sleep(1000); - } catch (InterruptedException e) { - Assert.fail("interrupted", e); + try { + getBlackboard().signalGate(gateToSignal); + getBlackboard().waitForGate(gateToWaitOn, 10, TimeUnit.SECONDS); + } catch (TimeoutException | InterruptedException e) { + throw new RuntimeException("failed", e); + } + ResultCollector collector = FunctionService.onMember(member).execute(new TestFunction()); + // wait the function to lock the lock on member. + collector.getResult(); + } finally { + lock.unlock(); } - ResultCollector collector = FunctionService.onMember(member).execute(new TestFunction()); - // wait the function to lock the lock on member. - collector.getResult(); - lock.unlock(); } }); } @@ -244,14 +259,19 @@ public class GemFireDeadlockDetectorDUnitTest extends JUnit4CacheTestCase { public void execute(FunctionContext context) { + boolean acquired = false; try { stuckThreads.add(Thread.currentThread()); - lock.tryLock(LOCK_WAIT_TIME, TimeUnit.SECONDS); + acquired = lock.tryLock(LOCK_WAIT_TIME, TimeUnit.SECONDS); } catch (InterruptedException e) { - // ingore + // ignore + } finally { + if (acquired) { + lock.unlock(); + } + stuckThreads.remove(Thread.currentThread()); + context.getResultSender().lastResult(null); } - stuckThreads.remove(Thread.currentThread()); - context.getResultSender().lastResult(null); } public String getId() { http://git-wip-us.apache.org/repos/asf/geode/blob/50686b0b/geode-core/src/test/java/org/apache/geode/test/dunit/DUnitBlackboard.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/DUnitBlackboard.java b/geode-core/src/test/java/org/apache/geode/test/dunit/DUnitBlackboard.java index a097cd4..62c92bd 100755 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/DUnitBlackboard.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/DUnitBlackboard.java @@ -56,6 +56,7 @@ public class DUnitBlackboard { * signals a boolean gate */ public void signalGate(String gateName) { + // System.out.println(Thread.currentThread().getName()+": signaling gate " + gateName); try { blackboard.signalGate(gateName); } catch (RemoteException e) { @@ -68,6 +69,7 @@ public class DUnitBlackboard { */ public void waitForGate(String gateName, long timeout, TimeUnit units) throws TimeoutException, InterruptedException { + // System.out.println(Thread.currentThread().getName()+": waiting for gate " + gateName); try { blackboard.waitForGate(gateName, timeout, units); } catch (RemoteException e) { @@ -77,6 +79,17 @@ public class DUnitBlackboard { } /** + * clear a gate + */ + public void clearGate(String gateName) { + try { + blackboard.clearGate(gateName); + } catch (RemoteException e) { + throw new RuntimeException("remote call failed", e); + } + } + + /** * test to see if a gate has been signeled */ public boolean isGateSignaled(String gateName) { http://git-wip-us.apache.org/repos/asf/geode/blob/50686b0b/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboard.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboard.java b/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboard.java index 63f833b..bc5b9b7 100755 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboard.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboard.java @@ -50,6 +50,11 @@ public interface InternalBlackboard extends Remote, Serializable { throws RemoteException, TimeoutException, InterruptedException; /** + * clears a gate + */ + void clearGate(String gateName) throws RemoteException; + + /** * test to see if a gate has been signeled */ boolean isGateSignaled(String gateName) throws RemoteException; http://git-wip-us.apache.org/repos/asf/geode/blob/50686b0b/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboardImpl.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboardImpl.java b/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboardImpl.java index e7657ed..feeae15 100755 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboardImpl.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/internal/InternalBlackboardImpl.java @@ -79,6 +79,11 @@ public class InternalBlackboardImpl extends UnicastRemoteObject implements Inter } @Override + public void clearGate(final String gateName) throws RemoteException { + gates.remove(gateName); + } + + @Override public void signalGate(final String gateName) throws RemoteException { gates.put(gateName, Boolean.TRUE); }