Repository: geode Updated Branches: refs/heads/feature/GEM-1483 1ceb97c7a -> 40fb5fdf9 (forced update)
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/GEM-1483 Commit: 987f20326d00a60429b90149e32ba8b82b199634 Parents: 02adb28 Author: Nick Reich <nre...@pivotal.io> Authored: Wed Jul 12 09:54:07 2017 -0700 Committer: Darrel Schneider <dschnei...@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 }
