geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject geode git commit: GEODE-3194: cleanup disk store on failed initial recovery
Date Wed, 12 Jul 2017 17:53:47 GMT
Repository: geode
Updated Branches:
  refs/heads/develop 02adb28c0 -> 987f20326


GEODE-3194: cleanup disk store on failed initial recovery

This closes #627


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

Branch: refs/heads/develop
Commit: 987f20326d00a60429b90149e32ba8b82b199634
Parents: 02adb28
Author: Nick Reich <nreich@pivotal.io>
Authored: Wed Jul 12 09:54:07 2017 -0700
Committer: Darrel Schneider <dschneider@pivotal.io>
Committed: Wed Jul 12 10:52:45 2017 -0700

----------------------------------------------------------------------
 .../internal/cache/DiskStoreFactoryImpl.java    | 18 ++++++++++++++++--
 .../cache/DiskStoreFactoryJUnitTest.java        | 20 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/987f2032/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreFactoryImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreFactoryImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreFactoryImpl.java
index 7a7044b..0288ef1 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreFactoryImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreFactoryImpl.java
@@ -119,7 +119,7 @@ public class DiskStoreFactoryImpl implements DiskStoreFactory {
       DiskStoreImpl ds =
           new DiskStoreImpl(this.cache, this.attrs, true/* ownedByRegion */, internalRegionArgs);
       if (isOwnedByPR) {
-        ds.doInitialRecovery();
+        initializeDiskStore(ds);
       }
       this.cache.addRegionOwnedDiskStore(ds);
       return ds;
@@ -141,7 +141,7 @@ public class DiskStoreFactoryImpl implements DiskStoreFactory {
           // Added for M&M
           this.cache.getInternalDistributedSystem()
               .handleResourceEvent(ResourceEvent.DISKSTORE_CREATE, dsi);
-          dsi.doInitialRecovery();
+          initializeDiskStore(dsi);
           this.cache.addDiskStore(dsi);
           if (registry != null) {
             registry.creatingDiskStore(dsi);
@@ -168,6 +168,20 @@ public class DiskStoreFactoryImpl implements DiskStoreFactory {
     return result;
   }
 
+  /**
+   * Protected for testing purposes. If during the initial recovery for the disk store, an
uncaught
+   * exception is thrown, the disk store will not be in a valid state. In this case, we want
to
+   * ensure the resources of the disk store are cleaned up.
+   */
+  protected void initializeDiskStore(DiskStoreImpl diskStore) {
+    try {
+      diskStore.doInitialRecovery();
+    } catch (RuntimeException e) {
+      diskStore.close();
+      throw e;
+    }
+  }
+
   private DiskStore findExisting(String name) {
     DiskStore existing = null;
     if (this.cache instanceof GemFireCacheImpl) {

http://git-wip-us.apache.org/repos/asf/geode/blob/987f2032/geode-core/src/test/java/org/apache/geode/internal/cache/DiskStoreFactoryJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/DiskStoreFactoryJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/DiskStoreFactoryJUnitTest.java
index ea0baa2..fdbe184 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/DiskStoreFactoryJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/DiskStoreFactoryJUnitTest.java
@@ -16,6 +16,11 @@ package org.apache.geode.internal.cache;
 
 import static org.apache.geode.distributed.ConfigurationProperties.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import java.io.File;
 import java.io.FilenameFilter;
@@ -424,6 +429,21 @@ public class DiskStoreFactoryJUnitTest {
     removeFiles(diskStore);
   }
 
+  @Test
+  public void failedDiskStoreInitialRecoveryCleansUpDiskStore() {
+    DiskStoreFactory dsf = cache.createDiskStoreFactory();
+    DiskStoreImpl diskStore = mock(DiskStoreImpl.class);
+    doThrow(RuntimeException.class).when(diskStore).doInitialRecovery();
+    boolean threwException = false;
+    try {
+      ((DiskStoreFactoryImpl) dsf).initializeDiskStore(diskStore);
+    } catch (RuntimeException e) {
+      threwException = true;
+    }
+    assertTrue(threwException);
+    verify(diskStore, times(1)).close();
+  }
+
   // setDiskDirs and setDiskDirsAndSizes are tested in DiskRegionIllegalArguementsJUnitTest
   // also setDiskUsageWarningPercentage and setDiskUsageCriticalPercentage
 }


Mime
View raw message