You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by la...@apache.org on 2017/07/19 18:04:29 UTC
[11/51] [abbrv] geode git commit: GEODE-3194: cleanup disk store on
failed initial recovery
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/feature/GEODE-3113
Commit: 987f20326d00a60429b90149e32ba8b82b199634
Parents: 02adb28
Author: Nick Reich <nr...@pivotal.io>
Authored: Wed Jul 12 09:54:07 2017 -0700
Committer: Darrel Schneider <ds...@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
}