geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From upthewatersp...@apache.org
Subject incubator-geode git commit: GEODE-601: Fix intermittent exception in DiskRegionJUnitTest.
Date Tue, 19 Jan 2016 23:02:10 GMT
Repository: incubator-geode
Updated Branches:
  refs/heads/develop f45f3a995 -> c465541d9


GEODE-601: Fix intermittent exception in DiskRegionJUnitTest.

An expected DiskAccessException causes the cache to close
asynchronously. That leads to a race condition during test tearDown,
between checking that the cache is closed and it's access. Now wait
for the cache to close before tearDown.
Refactors:
 Moved putSuccessful and exceptionOccurred inside Puts
 Remove unnecessary thread creation for Puts in DiskRegionJUnitTest
 Expose _testHandleDiskAccessException through method

This closes #72


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

Branch: refs/heads/develop
Commit: c465541d919652d3d4a1305ea6ddcd7c31513188
Parents: f45f3a9
Author: Shreedhar Hardikar <shardikar@pivotal.io>
Authored: Thu Jan 7 14:01:21 2016 -0800
Committer: Dan Smith <upthewaterspout@apache.org>
Committed: Tue Jan 19 15:01:44 2016 -0800

----------------------------------------------------------------------
 .../gemfire/internal/cache/DiskStoreImpl.java   |   4 +-
 .../internal/cache/Bug39079DUnitTest.java       |   4 +-
 .../internal/cache/DiskRegionJUnitTest.java     | 172 +++++++------------
 .../internal/cache/DiskRegionTestingBase.java   |   1 -
 4 files changed, 66 insertions(+), 115 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c465541d/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskStoreImpl.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskStoreImpl.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskStoreImpl.java
index 99524d9..e4ef21d 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskStoreImpl.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskStoreImpl.java
@@ -368,7 +368,7 @@ public class DiskStoreImpl implements DiskStore {
    */
   private DiskStoreID diskStoreID;
 
-  final CountDownLatch _testHandleDiskAccessException = new CountDownLatch(1);
+  private final CountDownLatch _testHandleDiskAccessException = new CountDownLatch(1);
   
   private final ThreadPoolExecutor diskStoreTaskPool;
   
@@ -2311,7 +2311,7 @@ public class DiskStoreImpl implements DiskStore {
     close(false);
   }
 
-  void waitForClose() {
+  protected void waitForClose() {
     if (diskException.get() != null) {
       try {
         _testHandleDiskAccessException.await();

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c465541d/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/Bug39079DUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/Bug39079DUnitTest.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/Bug39079DUnitTest.java
index f17d896..4927abe 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/Bug39079DUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/Bug39079DUnitTest.java
@@ -356,7 +356,7 @@ public class Bug39079DUnitTest extends CacheTestCase {
     assertNotNull(gemfirecache);
   }
   
-  private static void validateRuningBridgeServerList() throws Exception{
+  private static void validateRuningBridgeServerList(){
     /*Region region = gemfirecache.getRegion(Region.SEPARATOR + REGION_NAME);
     assertNotNull(region);*/
     try {        
@@ -374,7 +374,7 @@ public class Bug39079DUnitTest extends CacheTestCase {
         fail("test failed due to ", e);
       }
       
-      ((LocalRegion)region).getDiskRegion().getDiskStore()._testHandleDiskAccessException.await();
+      ((LocalRegion) region).getDiskStore().waitForClose();
       assertTrue(region.getRegionService().isClosed());
       
       region = null;

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c465541d/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionJUnitTest.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionJUnitTest.java
index cb9d3ff..e3b5ab0 100755
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionJUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionJUnitTest.java
@@ -548,7 +548,6 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
    * 
    * @author ashahid
    */
-  protected volatile  boolean[] putSuccessfull = new boolean[3];
 
   @Test
   public void testSingleDirectoryNotHanging()
@@ -568,31 +567,21 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
         diskRegionProperties, Scope.LOCAL);
 
     Puts puts = new Puts(region);
+    puts.performPuts();
 
-    Thread thread1 = new Thread(puts);
-    thread1.setDaemon(true);
-    thread1.start();
-
-    long startTime = System.currentTimeMillis();
-    DistributedTestCase.join(thread1, 40 * 1000, null);
-    long interval = System.currentTimeMillis() - startTime;
-    if (interval > 40000) {
-      fail(" Test took too long in going to join, it should have exited before 40000 ms");
-    }
-
-    if (!putSuccessfull[0]) {
+    if (!puts.putSuccessful(0)) {
       fail(" first put did not succeed");
     }
 
-    if (!putSuccessfull[1]) {
+    if (!puts.putSuccessful(1)) {
       fail(" second put did not succeed");
     }
 
-    if (!putSuccessfull[2]) {
+    if (!puts.putSuccessful(2)) {
       fail(" third put did not succeed");
     }
 
-    if (exceptionOccured) {
+    if (puts.exceptionOccurred()) {
       fail(" Exception was not supposed to occur but did occur");
     }
     closeDown();
@@ -612,42 +601,21 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
         diskRegionProperties, Scope.LOCAL);
 
     Puts puts = new Puts(region);
-    Thread thread1 = new Thread(puts);
-    thread1.setDaemon(true);
-    thread1.start();
-    if (!putsHaveStarted) {
-      synchronized (puts) {
-        if (!putsHaveStarted) {
-          try {
-            puts.wait();
-          }
-          catch (InterruptedException e) {
-            fail("Unexpected interrupted exception");
-          }
-        }
-      }
-    }
-   
-    long startTime = System.currentTimeMillis();
-    DistributedTestCase.join(thread1, 9 * 1000, null);
-    long interval = System.currentTimeMillis() - startTime;
-    if (interval > 9000) {
-      fail(" Test took too long in going to join, it should have exited before 9000 ms");
-    }
+    puts.performPuts();
 
-    if (!putSuccessfull[0]) {
+    if (!puts.putSuccessful(0)) {
       fail(" first put did not succeed");
     }
 
-    if (!putSuccessfull[1]) {
+    if (!puts.putSuccessful(1)) {
       fail(" second put did not succeed");
     }
 
-    if (!putSuccessfull[2]) {
+    if (!puts.putSuccessful(2)) {
       fail(" third put did not succeed");
     }
 
-    if (exceptionOccured) {
+    if (puts.exceptionOccurred()) {
       fail(" Exception was not supposed to occur but did occur");
     }
 
@@ -675,46 +643,25 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
     }
 
     Puts puts = new Puts(region, 1026);
+    puts.performPuts();
 
-    Thread thread1 = new Thread(puts);
-    thread1.setDaemon(true);
-    thread1.start();
-
-    
-    if (!putsHaveStarted) {
-      synchronized (puts) {
-        if (!putsHaveStarted) {
-          try {
-            puts.wait();
-          }
-          catch (InterruptedException e) {
-            fail("Unexpected interrupted exception");
-          }
-        }
-      }
-    }
-    long startTime = System.currentTimeMillis();
-    DistributedTestCase.join(thread1, 33 * 1000, null);
-    long interval = System.currentTimeMillis() - startTime;
-    if (interval > 33000) {
-      fail(" Test took too long in going to join, it should have exited before 33000 ms");
-    }
-
-    if (!exceptionOccured) {
+    if (!puts.exceptionOccurred()) {
       fail(" Exception was supposed to occur but did not occur");
     }
-    if (putSuccessfull[0]) {
+    if (puts.putSuccessful(0)) {
       fail(" first put did succeed when it was not supposed to");
     }
 
-    if (putSuccessfull[1]) {
+    if (puts.putSuccessful(1)) {
       fail(" second put did  succeed  when it was not supposed to");
     }
 
-    if (putSuccessfull[2]) {
+    if (puts.putSuccessful(2)) {
       fail(" third put did  succeed  when it was not supposed to");
     }
 
+    // if the exception occurred then the region should be closed already
+    ((LocalRegion) region).getDiskStore().waitForClose();
   }
 
   /**
@@ -788,6 +735,8 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
 
     private int dataSize = 1024;
     private Region region;
+    private volatile boolean[] putSuccessful = new boolean[3];
+    private volatile boolean exceptionOccurred = false;
 
     Puts(Region region) {
       this.region = region;
@@ -796,15 +745,24 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
       this.region = region;
       this.dataSize = dataSize;
     }
-      
 
-    public void run()
-    {
-     
-      exceptionOccured = false;
-      putSuccessfull[0] = false;
-      putSuccessfull[1] = false;
-      putSuccessfull[2] = false;
+    public boolean exceptionOccurred() {
+      return exceptionOccurred;
+    }
+
+    public boolean putSuccessful(int index){
+      return putSuccessful[index];
+    }
+
+    public void run() {
+      performPuts();
+    }
+ 
+    public void performPuts(){
+      exceptionOccurred = false;
+      putSuccessful[0] = false;
+      putSuccessful[1] = false;
+      putSuccessful[2] = false;
 
       try {
         byte[] bytes = new byte[this.dataSize];
@@ -813,14 +771,14 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
           this.notify();
         }
         region.put("1", bytes);
-        putSuccessfull[0] = true;
+        putSuccessful[0] = true;
         region.put("2", bytes);
-        putSuccessfull[1] = true;
+        putSuccessful[1] = true;
         region.put("3", bytes);
-        putSuccessfull[2] = true;
+        putSuccessful[2] = true;
       }
       catch (DiskAccessException e) {
-        exceptionOccured = true;
+        exceptionOccurred = true;
       }
     }
   }
@@ -835,8 +793,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
     File dir = new File("testSingleDirectoryNotHanging");
     dir.mkdir();
     dir.deleteOnExit();
-    File[] dirs = new File[1];
-    dirs[0] = dir;
+    File[] dirs = new File[] {dir};
     int[] dirSizes = { 2048 };
     diskRegionProperties.setDiskDirsAndSizes(dirs, dirSizes);
     diskRegionProperties.setMaxOplogSize(2097152);
@@ -845,33 +802,30 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
         diskRegionProperties, Scope.LOCAL);
 
     Puts puts = new Puts(region);
+    puts.performPuts();
 
-    Thread thread1 = new Thread(puts);
-    thread1.setDaemon(true);
-    thread1.start();
-    DistributedTestCase.join(thread1, 30 * 1000, null);
-
-    if (!putSuccessfull[0]) {
+    if (!puts.putSuccessful(0)) {
       fail(" first put did not succeed");
     }
 
-    if (putSuccessfull[1]) {
+    if (puts.putSuccessful(1)) {
       fail(" second put should not succeed");
     }
 
-    if (!exceptionOccured) {
+    if (!puts.exceptionOccurred()) {
       fail(" Exception was supposed to occur but did not occur");
     }
-    closeDown();
+    // if the exception occurred then the region should be closed already
+    ((LocalRegion) region).getDiskStore().waitForClose();
 
+    closeDown();
   }
 
   /**
    * DiskRegDiskAccessExceptionTest : Disk region test for DiskAccessException.
-   * @throws InterruptedException 
    */
   @Test
-  public void testDiskFullExcep() throws InterruptedException
+  public void testDiskFullExcep()
   {
     int[] diskDirSize1 = new int[4];
     diskDirSize1[0] = (2048 + 500);
@@ -921,9 +875,8 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
                      + "</ExpectedException>");
     }
 
-    // if the exception Occurred then the region should be closed already
-    ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
-    ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+    // if the exception occurred then the region should be closed already
+    ((LocalRegion) region).getDiskStore().waitForClose();
     assertEquals(true, cache.isClosed());
   }
 
@@ -981,10 +934,9 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
   }
   /**
    * DiskRegDiskAccessExceptionTest : Disk region test for DiskAccessException.
-   * @throws InterruptedException 
    */
   @Test
-  public void testDiskFullExcepOverflowOnly() throws InterruptedException
+  public void testDiskFullExcepOverflowOnly()
   {
     int[] diskDirSize1 = new int[4];
     diskDirSize1[0] = (2048 + 500);
@@ -1038,8 +990,8 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
                      + "</ExpectedException>");
     }
 
-    // if the exception Occurred then the region should be closed already
-    ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+    // if the exception occurred then the region should be closed already
+    ((LocalRegion) region).getDiskStore().waitForClose();
     assertEquals(true, cache.isClosed());
   }
   
@@ -1983,7 +1935,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
       // OK
     }
 
-    ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+    ((LocalRegion) region).getDiskStore().waitForClose();
     assertTrue(cache.isClosed());
     region = null;
   }
@@ -2047,7 +1999,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
       // OK
     }
 
-    ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+    ((LocalRegion) region).getDiskStore().waitForClose();
     assertTrue(cache.isClosed());
     region = null;
   }
@@ -2113,7 +2065,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
     catch (DiskAccessException dae) {
       // OK
     }
-    ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+    ((LocalRegion) region).getDiskStore().waitForClose();
     assertTrue(cache.isClosed());
     region = null;
   }
@@ -2178,7 +2130,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
       // OK
     }
 
-    ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+    ((LocalRegion) region).getDiskStore().waitForClose();
     assertTrue(cache.isClosed());
     region = null;
   }
@@ -2212,7 +2164,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
       catch (DiskAccessException dae) {
         //OK
       }        
-      ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+      ((LocalRegion) region).getDiskStore().waitForClose();
       assertTrue(cache.isClosed());
       region = null;
   }
@@ -2245,7 +2197,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
       catch (DiskAccessException dae) {
         //OK
       }       
-      ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+      ((LocalRegion) region).getDiskStore().waitForClose();
       assertTrue(cache.isClosed());
       region = null;
   }
@@ -2326,7 +2278,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
         }                
       }, 5000, 500, true);
       
-      ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+      ((LocalRegion) region).getDiskStore().waitForClose();
       assertTrue(cache.isClosed());
       region = null;
   }
@@ -2364,7 +2316,7 @@ public class DiskRegionJUnitTest extends DiskRegionTestingBase
       }catch(DiskAccessException dae) {
         //OK expected
       }
-      ((LocalRegion) region).getDiskStore()._testHandleDiskAccessException.await();
+      ((LocalRegion) region).getDiskStore().waitForClose();
       assertTrue(cache.isClosed());        
       region = null;
       List bsRunning = cache.getCacheServers();

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c465541d/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionTestingBase.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionTestingBase.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionTestingBase.java
index 8a425ee..d0911b8 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionTestingBase.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/DiskRegionTestingBase.java
@@ -134,7 +134,6 @@ public class DiskRegionTestingBase
       if (cache != null && !cache.isClosed()) {
         for (Iterator itr = cache.rootRegions().iterator(); itr.hasNext();) {
           Region root = (Region)itr.next();
-//          String name = root.name.getMethodName();
           if(root.isDestroyed() || root instanceof HARegion) {
             continue;
           }


Mime
View raw message