geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject [07/13] incubator-geode git commit: GEODE-153: fix race in testRemoteCacheListener
Date Fri, 24 Jul 2015 00:16:18 GMT
GEODE-153: fix race in testRemoteCacheListener

Both testRemoteCacheListener and testRemoteCacheListenerInSubregion used sleeps
to try to avoid no-ack races. Both tests now use a wait criteria.


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

Branch: refs/heads/feature/GEODE-148
Commit: 3bfee73c4e7f963400bde99c90fbeb0c604d6f94
Parents: dd4ab5f
Author: Darrel Schneider <dschneider@pivotal.io>
Authored: Thu Jul 23 15:37:57 2015 -0700
Committer: Darrel Schneider <dschneider@pivotal.io>
Committed: Thu Jul 23 15:37:57 2015 -0700

----------------------------------------------------------------------
 .../gemfire/cache30/MultiVMRegionTestCase.java  | 52 +++++++-------------
 .../gemfire/cache30/TestCacheCallback.java      | 13 +++--
 2 files changed, 27 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/3bfee73c/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
index dfbc82d..97208ca 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
@@ -1004,18 +1004,13 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         };
 
     Host host = Host.getHost(0);
-    VM vm0 = host.getVM(0);
-
-    vm0.invoke(create);
-
     int vmCount = host.getVMCount();
-    for (int i = 1; i < vmCount; i++) {
+    for (int i = 0; i < vmCount; i++) {
       VM vm = host.getVM(i);
       vm.invoke(create);
     }
 
-    Thread.sleep(250);
-
+    pause();
 
     SerializableRunnable invalidate =
       new CacheSerializableRunnable("Invalidate Entry") {
@@ -1047,6 +1042,7 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
       vm.invoke(verify);
     }
 
+    // Why is this long 2.5 second pause needed on no-ack?
     pauseIfNecessary(2500);  // wait for messages to acquiesce before tearing down
     getLogWriter().info("Tearing down...");
   }
@@ -1211,7 +1207,14 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         }
       });
 
-    pauseIfNecessary();
+    // I see no reason to pause here.
+    // The test used to pause here but only if no-ack.
+    // But we have no operations to wait for.
+    // The last thing we did was install a listener in vm1
+    // and it is possible that vm0 does not yet know we have
+    // a listener but for this test it does not matter.
+    // So I'm commenting out the following pause:
+    //pauseIfNecessary();
 
     vm0.invoke(new CacheSerializableRunnable("Update") {
         public void run2() throws CacheException {
@@ -1221,11 +1224,9 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         }
       });
 
-    pauseIfNecessary();
-
     vm1.invoke(new CacheSerializableRunnable("Verify Update") {
         public void run2() throws CacheException {
-          assertTrue(listener.wasInvoked());
+          listener.waitForInvocation(3000, 10);
 
           // Setup listener for next test
           final Region region =
@@ -1266,11 +1267,9 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         }
       });
 
-    pause();
-
     vm1.invoke(new CacheSerializableRunnable("Verify Invalidate") {
         public void run2() throws CacheException {
-          assertTrue(listener.wasInvoked());
+          listener.waitForInvocation(3000, 10);
 
           // Setup listener for next test
           final Region region =
@@ -1306,11 +1305,9 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         }
       });
 
-    pause();
-
     vm1.invoke(new CacheSerializableRunnable("Verify Destroy") {
         public void run2() throws CacheException {
-          assertTrue(listener.wasInvoked());
+          listener.waitForInvocation(3000, 10);
 
           // Setup listener for next test
           final Region region =
@@ -1337,11 +1334,9 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         }
       });
 
-    pause();
-
     vm1.invoke(new CacheSerializableRunnable("Verify Invalidate Region") {
         public void run2() throws CacheException {
-          assertTrue(listener.wasInvoked());
+          listener.waitForInvocation(3000, 10);
 
           // Setup listener for next test
           final Region region =
@@ -1368,11 +1363,9 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         }
       });
 
-    pause();
-
     vm1.invoke(new CacheSerializableRunnable("Verify Destroy Region") {
         public void run2() throws CacheException {
-          assertTrue(listener.wasInvoked());
+          listener.waitForInvocation(3000, 10);
         }
       });
   }
@@ -1426,20 +1419,15 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         }
       });
 
-    pauseIfNecessary();
-
-
     vm0.invoke(new CacheSerializableRunnable("Invalidate Root Region") {
         public void run2() throws CacheException {
           getRootRegion().invalidateRegion(getSystem().getDistributedMember());
         }
       });
 
-    pauseIfNecessary();
-
     vm1.invoke(new CacheSerializableRunnable("Verify Invalidate Region") {
         public void run2() throws CacheException {
-          assertTrue(listener.wasInvoked());
+          listener.waitForInvocation(3000, 10);
 
           // Setup listener for next test
           final Region region =
@@ -1458,19 +1446,15 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase
{
         }
       });
 
-    pauseIfNecessary();
-
     vm0.invoke(new CacheSerializableRunnable("Destroy Root Region") {
         public void run2() throws CacheException {
           getRootRegion().destroyRegion(getSystem().getDistributedMember());
         }
       });
 
-    pauseIfNecessary();
-
     vm1.invoke(new CacheSerializableRunnable("Verify Destroy Region") {
         public void run2() throws CacheException {
-          assertTrue(listener.wasInvoked());
+          listener.waitForInvocation(3000, 10);
         }
       });
   }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/3bfee73c/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TestCacheCallback.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TestCacheCallback.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TestCacheCallback.java
index a252059..f420939 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TestCacheCallback.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TestCacheCallback.java
@@ -32,14 +32,16 @@ public abstract class TestCacheCallback implements CacheCallback {
   volatile protected Throwable callbackError = null;
 
   /**
-   * Returns wether or not one of this <code>CacheListener</code>
+   * Returns whether or not one of this <code>CacheListener</code>
    * methods was invoked.  Before returning, the <code>invoked</code>
    * flag is cleared.
    */
   public boolean wasInvoked() {
     checkForError();
     boolean value = this.invoked;
-    this.invoked = false;
+    if (value) {
+      this.invoked = false;
+    }
     return value;
   }
   /**
@@ -47,16 +49,19 @@ public abstract class TestCacheCallback implements CacheCallback {
    * Calls wasInvoked and returns its value
    */
   public boolean waitForInvocation(int timeoutMs) {
+    return waitForInvocation(timeoutMs, 200);
+  }
+  public boolean waitForInvocation(int timeoutMs, long interval) {
     if (!this.invoked) {
       WaitCriterion ev = new WaitCriterion() {
         public boolean done() {
           return invoked;
         }
         public String description() {
-          return null;
+          return "listener was never invoked";
         }
       };
-      DistributedTestCase.waitForCriterion(ev, timeoutMs, 200, true);
+      DistributedTestCase.waitForCriterion(ev, timeoutMs, interval, true);
     }
     return wasInvoked();
   }


Mime
View raw message