You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2017/04/21 23:42:40 UTC

[44/51] [abbrv] geode git commit: GEODE-576 & GEODE-516: GemFireDeadlockDetectorDUnitTest failures

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/GEODE-2097
Commit: 50686b0b44024d2bcbb4bea8a36ce3a40ac158c2
Parents: 5891ed7
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Fri Apr 21 09:05:07 2017 -0700
Committer: Bruce Schuchardt <bs...@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 a097cd4e..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);
   }