You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2017/08/19 00:10:34 UTC

[39/51] [abbrv] geode git commit: GEODE-3169: Decoupling of DiskStore and backups This closes #715 * move backup logic away from DiskStore and into BackupManager * refactor code into smaller methods * improve test code clarity

http://git-wip-us.apache.org/repos/asf/geode/blob/3bb6a221/geode-core/src/test/java/org/apache/geode/internal/cache/BackupJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/BackupJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/BackupJUnitTest.java
index caa2ce5..28dc662 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/BackupJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/BackupJUnitTest.java
@@ -23,18 +23,15 @@ import static org.junit.Assert.*;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.filefilter.DirectoryFileFilter;
 import org.apache.commons.io.filefilter.RegexFileFilter;
+
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.DataPolicy;
 import org.apache.geode.cache.DiskStore;
 import org.apache.geode.cache.DiskStoreFactory;
-import org.apache.geode.cache.DiskWriteAttributesFactory;
 import org.apache.geode.cache.EvictionAction;
 import org.apache.geode.cache.EvictionAttributes;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionFactory;
-import org.apache.geode.distributed.DistributedSystem;
-import org.apache.geode.internal.cache.persistence.BackupManager;
-import org.apache.geode.internal.cache.persistence.RestoreScript;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.junit.After;
 import org.junit.Before;
@@ -54,16 +51,17 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Properties;
 import java.util.Random;
+import java.util.concurrent.CompletableFuture;
 
 @Category(IntegrationTest.class)
 public class BackupJUnitTest {
 
-  protected GemFireCacheImpl cache = null;
+  private static final String DISK_STORE_NAME = "diskStore";
+  private GemFireCacheImpl cache = null;
   private File tmpDir;
-  protected File cacheXmlFile;
+  private File cacheXmlFile;
 
-  protected DistributedSystem ds = null;
-  protected Properties props = new Properties();
+  private Properties props = new Properties();
 
   private File backupDir;
   private File[] diskDirs;
@@ -103,7 +101,6 @@ public class BackupJUnitTest {
 
   private void createCache() throws IOException {
     cache = (GemFireCacheImpl) new CacheFactory(props).create();
-    ds = cache.getDistributedSystem();
   }
 
   @After
@@ -123,33 +120,26 @@ public class BackupJUnitTest {
 
   @Test
   public void testBackupAndRecover() throws IOException, InterruptedException {
-    backupAndRecover(new RegionCreator() {
-      public Region createRegion() {
-        DiskStoreImpl ds = createDiskStore();
-        return BackupJUnitTest.this.createRegion();
-      }
+    backupAndRecover(() -> {
+      createDiskStore();
+      return BackupJUnitTest.this.createRegion();
     });
   }
 
   @Test
   public void testBackupAndRecoverOldConfig() throws IOException, InterruptedException {
-    backupAndRecover(new RegionCreator() {
-      public Region createRegion() {
-        DiskStoreImpl ds = createDiskStore();
-        RegionFactory rf = new RegionFactory();
-        rf.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
-        rf.setDiskDirs(diskDirs);
-        DiskWriteAttributesFactory daf = new DiskWriteAttributesFactory();
-        daf.setMaxOplogSize(1);
-        rf.setDiskWriteAttributes(daf.create());
-        return rf.create("region");
-      }
+    backupAndRecover(() -> {
+      createDiskStore();
+      RegionFactory regionFactory = cache.createRegionFactory();
+      regionFactory.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
+      regionFactory.setDiskStoreName(DISK_STORE_NAME);
+      return regionFactory.create("region");
     });
   }
 
-  public void backupAndRecover(RegionCreator regionFactory)
+  private void backupAndRecover(RegionCreator regionFactory)
       throws IOException, InterruptedException {
-    Region region = regionFactory.createRegion();
+    Region<Object, Object> region = regionFactory.createRegion();
 
     // Put enough data to roll some oplogs
     for (int i = 0; i < 1024; i++) {
@@ -193,8 +183,8 @@ public class BackupJUnitTest {
 
     BackupManager backup =
         cache.startBackup(cache.getInternalDistributedSystem().getDistributedMember());
-    backup.prepareBackup();
-    backup.finishBackup(backupDir, null, false);
+    backup.prepareForBackup();
+    backup.doBackup(backupDir, null, false);
 
     // Put another key to make sure we restore
     // from a backup that doesn't contain this key
@@ -238,19 +228,19 @@ public class BackupJUnitTest {
 
   @Test
   public void testBackupEmptyDiskStore() throws IOException, InterruptedException {
-    DiskStoreImpl ds = createDiskStore();
+    createDiskStore();
 
     BackupManager backup =
         cache.startBackup(cache.getInternalDistributedSystem().getDistributedMember());
-    backup.prepareBackup();
-    backup.finishBackup(backupDir, null, false);
+    backup.prepareForBackup();
+    backup.doBackup(backupDir, null, false);
     assertEquals("No backup files should have been created", Collections.emptyList(),
         Arrays.asList(backupDir.list()));
   }
 
   @Test
   public void testBackupOverflowOnlyDiskStore() throws IOException, InterruptedException {
-    DiskStoreImpl ds = createDiskStore();
+    createDiskStore();
     Region region = createOverflowRegion();
     // Put another key to make sure we restore
     // from a backup that doesn't contain this key
@@ -258,8 +248,8 @@ public class BackupJUnitTest {
 
     BackupManager backup =
         cache.startBackup(cache.getInternalDistributedSystem().getDistributedMember());
-    backup.prepareBackup();
-    backup.finishBackup(backupDir, null, false);
+    backup.prepareForBackup();
+    backup.doBackup(backupDir, null, false);
 
 
     assertEquals("No backup files should have been created", Collections.emptyList(),
@@ -275,51 +265,54 @@ public class BackupJUnitTest {
     dsf.setAutoCompact(false);
     dsf.setAllowForceCompaction(true);
     dsf.setCompactionThreshold(20);
-    String name = "diskStore";
-    DiskStoreImpl ds = (DiskStoreImpl) dsf.create(name);
+    DiskStoreImpl ds = (DiskStoreImpl) dsf.create(DISK_STORE_NAME);
 
-    Region region = createRegion();
+    Region<Object, Object> region = createRegion();
 
     // Put enough data to roll some oplogs
     for (int i = 0; i < 1024; i++) {
       region.put(i, getBytes(i));
     }
 
-    RestoreScript script = new RestoreScript();
-    ds.startBackup(backupDir, null, script);
-
-    for (int i = 2; i < 1024; i++) {
-      assertTrue(region.destroy(i) != null);
-    }
-    assertTrue(ds.forceCompaction());
-    // Put another key to make sure we restore
-    // from a backup that doesn't contain this key
-    region.put("A", "A");
-
-    ds.finishBackup(
-        new BackupManager(cache.getInternalDistributedSystem().getDistributedMember(), cache));
-    script.generate(backupDir);
+    BackupManager backupManager =
+        cache.startBackup(cache.getInternalDistributedSystem().getDistributedMember());
+    backupManager.validateRequestingAdmin();
+    backupManager.prepareForBackup();
+    final Region theRegion = region;
+    final DiskStore theDiskStore = ds;
+    CompletableFuture.runAsync(() -> destroyAndCompact(theRegion, theDiskStore));
+    backupManager.doBackup(backupDir, null, false);
 
     cache.close();
     destroyDiskDirs();
     restoreBackup(false);
     createCache();
-    ds = createDiskStore();
+    createDiskStore();
     region = createRegion();
     validateEntriesExist(region, 0, 1024);
 
     assertNull(region.get("A"));
   }
 
+  private void destroyAndCompact(Region<Object, Object> region, DiskStore diskStore) {
+    for (int i = 2; i < 1024; i++) {
+      assertTrue(region.destroy(i) != null);
+    }
+    assertTrue(diskStore.forceCompaction());
+    // Put another key to make sure we restore
+    // from a backup that doesn't contain this key
+    region.put("A", "A");
+  }
+
   @Test
   public void testBackupCacheXml() throws Exception {
-    DiskStoreImpl ds = createDiskStore();
+    createDiskStore();
     createRegion();
 
     BackupManager backup =
         cache.startBackup(cache.getInternalDistributedSystem().getDistributedMember());
-    backup.prepareBackup();
-    backup.finishBackup(backupDir, null, false);
+    backup.prepareForBackup();
+    backup.doBackup(backupDir, null, false);
     Collection<File> fileCollection = FileUtils.listFiles(backupDir,
         new RegexFileFilter("cache.xml"), DirectoryFileFilter.DIRECTORY);
     assertEquals(1, fileCollection.size());
@@ -337,12 +330,9 @@ public class BackupJUnitTest {
     // The cache xml file should be small enough to fit in one byte array
     int size = (int) file.length();
     byte[] contents = new byte[size];
-    FileInputStream fis = new FileInputStream(file);
-    try {
+    try (FileInputStream fis = new FileInputStream(file)) {
       assertEquals(size, fis.read(contents));
       assertEquals(-1, fis.read());
-    } finally {
-      fis.close();
     }
     return contents;
   }
@@ -406,36 +396,35 @@ public class BackupJUnitTest {
 
   }
 
-  protected Region createRegion() {
-    RegionFactory rf = new RegionFactory();
-    rf.setDiskStoreName("diskStore");
-    rf.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
-    return rf.create("region");
+  private Region createRegion() {
+    RegionFactory regionFactory = cache.createRegionFactory();
+    regionFactory.setDiskStoreName(DISK_STORE_NAME);
+    regionFactory.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
+    return regionFactory.create("region");
   }
 
   private Region createOverflowRegion() {
-    RegionFactory rf = new RegionFactory();
-    rf.setDiskStoreName("diskStore");
-    rf.setEvictionAttributes(
+    RegionFactory regionFactory = cache.createRegionFactory();
+    regionFactory.setDiskStoreName(DISK_STORE_NAME);
+    regionFactory.setEvictionAttributes(
         EvictionAttributes.createLIFOEntryAttributes(1, EvictionAction.OVERFLOW_TO_DISK));
-    rf.setDataPolicy(DataPolicy.NORMAL);
-    return rf.create("region");
+    regionFactory.setDataPolicy(DataPolicy.NORMAL);
+    return regionFactory.create("region");
   }
 
   private DiskStore findDiskStore() {
-    return cache.findDiskStore("diskStore");
+    return cache.findDiskStore(DISK_STORE_NAME);
   }
 
-  private DiskStoreImpl createDiskStore() {
-    DiskStoreFactory dsf = cache.createDiskStoreFactory();
-    dsf.setDiskDirs(diskDirs);
-    dsf.setMaxOplogSize(1);
-    String name = "diskStore";
-    return (DiskStoreImpl) dsf.create(name);
+  private void createDiskStore() {
+    DiskStoreFactory diskStoreFactory = cache.createDiskStoreFactory();
+    diskStoreFactory.setDiskDirs(diskDirs);
+    diskStoreFactory.setMaxOplogSize(1);
+    diskStoreFactory.create(DISK_STORE_NAME);
   }
 
   private interface RegionCreator {
-    Region createRegion();
+    Region<Object, Object> createRegion();
   }
 
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/3bb6a221/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java
index ee3d7f7..f31f17b 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java
@@ -55,7 +55,6 @@ import org.apache.geode.distributed.DistributedSystem;
 import org.apache.geode.internal.ClassBuilder;
 import org.apache.geode.internal.ClassPathLoader;
 import org.apache.geode.internal.DeployedJar;
-import org.apache.geode.internal.cache.persistence.BackupManager;
 import org.apache.geode.internal.util.IOUtils;
 import org.apache.geode.internal.util.TransformUtils;
 import org.apache.geode.test.dunit.Host;
@@ -615,7 +614,7 @@ public class IncrementalBackupDUnitTest extends JUnit4CacheTestCase {
     File backupDir = getBackupDirForMember(getBaselineDir(), getMemberId(vm));
     assertTrue(backupDir.exists());
 
-    File incomplete = new File(backupDir, BackupManager.INCOMPLETE_BACKUP);
+    File incomplete = new File(backupDir, BackupManager.INCOMPLETE_BACKUP_FILE);
     incomplete.createNewFile();
   }
 

http://git-wip-us.apache.org/repos/asf/geode/blob/3bb6a221/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/BackupPrepareAndFinishMsgDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/BackupPrepareAndFinishMsgDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/BackupPrepareAndFinishMsgDUnitTest.java
index 39c5c3c..e0fea77 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/BackupPrepareAndFinishMsgDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/BackupPrepareAndFinishMsgDUnitTest.java
@@ -22,11 +22,18 @@ import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.stream.Collectors;
 
 import org.apache.geode.admin.internal.FinishBackupRequest;
 import org.apache.geode.admin.internal.PrepareBackupRequest;
@@ -46,490 +53,151 @@ import org.apache.geode.distributed.internal.DM;
 import org.apache.geode.internal.cache.BackupLock;
 import org.apache.geode.internal.cache.DiskStoreImpl;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.test.junit.categories.DistributedTest;
 import org.awaitility.Awaitility;
-import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 @Category({DistributedTest.class})
-public class BackupPrepareAndFinishMsgDUnitTest extends CacheTestCase {
+public abstract class BackupPrepareAndFinishMsgDUnitTest extends CacheTestCase {
   // Although this test does not make use of other members, the current member needs to be
   // a distributed member (rather than local) because it sends prepare and finish backup messages
-  File[] diskDirs = null;
+  private static final String TEST_REGION_NAME = "TestRegion";
+  private File[] diskDirs = null;
   private int waitingForBackupLockCount = 0;
+  private Region<Integer, Integer> region;
 
-  @After
-  public void after() throws Exception {
-    waitingForBackupLockCount = 0;
-    diskDirs = null;
-  }
-
-  @Test
-  public void testCreateWithParReg() throws Throwable {
-    doCreate(RegionShortcut.PARTITION_PERSISTENT, true);
-  }
-
-  @Test
-  public void testCreateWithReplicate() throws Throwable {
-    doCreate(RegionShortcut.REPLICATE_PERSISTENT, true);
-  }
-
-  @Test
-  public void testPutAsCreateWithParReg() throws Throwable {
-    doCreate(RegionShortcut.PARTITION_PERSISTENT, false);
-  }
-
-  @Test
-  public void testPutAsCreateWithReplicate() throws Throwable {
-    doCreate(RegionShortcut.REPLICATE_PERSISTENT, false);
-  }
-
-  @Test
-  public void testUpdateWithParReg() throws Throwable {
-    doUpdate(RegionShortcut.PARTITION_PERSISTENT);
-  }
-
-  @Test
-  public void testUpdateWithReplicate() throws Throwable {
-    doUpdate(RegionShortcut.REPLICATE_PERSISTENT);
-  }
-
-  @Test
-  public void testInvalidateWithParReg() throws Throwable {
-    doInvalidate(RegionShortcut.PARTITION_PERSISTENT);
-  }
-
-  @Test
-  public void testInvalidateWithReplicate() throws Throwable {
-    doInvalidate(RegionShortcut.REPLICATE_PERSISTENT);
-  }
-
-  @Test
-  public void testDestroyWithParReg() throws Throwable {
-    doDestroy(RegionShortcut.PARTITION_PERSISTENT);
-  }
-
-  @Test
-  public void testDestroyWithReplicate() throws Throwable {
-    doDestroy(RegionShortcut.REPLICATE_PERSISTENT);
-  }
-
-  @Test
-  public void testGetWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "get");
-  }
-
-  @Test
-  public void testGetWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "get");
-  }
-
-  @Test
-  public void testContainsKeyWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "containsKey");
-  }
-
-  @Test
-  public void testContainsKeyWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "containsKey");
-  }
-
-  @Test
-  public void testContainsValueWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "containsValue");
-  }
-
-  @Test
-  public void testContainsValueWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "containsValue");
-  }
-
-  @Test
-  public void testContainsValueForKeyWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "containsValueForKey");
-  }
-
-  @Test
-  public void testContainsValueForKeyWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "containsValueForKey");
-  }
-
-  @Test
-  public void testEntrySetWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "entrySet");
-  }
-
-  @Test
-  public void testEntrySetWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "entrySet");
-  }
-
-  @Test
-  public void testGetAllWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "getAll");
-  }
-
-  @Test
-  public void testGetAllWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "getAll");
-  }
-
-  @Test
-  public void testGetEntryWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "getEntry");
-  }
-
-  @Test
-  public void testGetEntryWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "getEntry");
-  }
-
-  @Test
-  public void testIsEmptyWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "isEmpty");
-  }
-
-  @Test
-  public void testIsEmptyWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "isEmpty");
-  }
-
-  @Test
-  public void testKeySetWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "keySet");
-  }
-
-  @Test
-  public void testKeySetWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "keySet");
-  }
-
-  @Test
-  public void testSizeWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "size");
-  }
+  protected abstract Region<Integer, Integer> createRegion();
 
-  @Test
-  public void testSizeWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "size");
+  @Before
+  public void setup() {
+    region = createRegion();
   }
 
   @Test
-  public void testValuesWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "values");
+  public void createWaitsForBackupTest() throws Throwable {
+    doActionAndVerifyWaitForBackup(() -> region.create(1, 1));
+    verifyKeyValuePair(1, 1);
   }
 
   @Test
-  public void testValuesWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "values");
+  public void putThatCreatesWaitsForBackupTest() throws Throwable {
+    doActionAndVerifyWaitForBackup(() -> region.put(1, 1));
+    verifyKeyValuePair(1, 1);
   }
 
   @Test
-  public void testQueryWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "query");
+  public void putWaitsForBackupTest() throws Throwable {
+    region.put(1, 1);
+    doActionAndVerifyWaitForBackup(() -> region.put(1, 2));
+    verifyKeyValuePair(1, 2);
   }
 
   @Test
-  public void testQueryWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "query");
+  public void invalidateWaitsForBackupTest() throws Throwable {
+    region.put(1, 1);
+    doActionAndVerifyWaitForBackup(() -> region.invalidate(1));
+    verifyKeyValuePair(1, null);
   }
 
   @Test
-  public void testExistsValueWithParReg() throws Throwable {
-    doRead(RegionShortcut.PARTITION_PERSISTENT, "existsValue");
+  public void destroyWaitsForBackupTest() throws Throwable {
+    region.put(1, 1);
+    doActionAndVerifyWaitForBackup(() -> region.destroy(1));
+    assertFalse(region.containsKey(1));
   }
 
   @Test
-  public void testExistsValueWithReplicate() throws Throwable {
-    doRead(RegionShortcut.REPLICATE_PERSISTENT, "existsValue");
-  }
+  public void putAllWaitsForBackupTest() throws Throwable {
+    Map<Integer, Integer> entries = new HashMap<>();
+    entries.put(1, 1);
+    entries.put(2, 2);
 
-  @Test
-  public void testPutAllWithParReg() throws Throwable {
-    doPutAll(RegionShortcut.PARTITION_PERSISTENT);
+    doActionAndVerifyWaitForBackup(() -> region.putAll(entries));
+    verifyKeyValuePair(1, 1);
+    verifyKeyValuePair(2, 2);
   }
 
   @Test
-  public void testPutAllWithReplicate() throws Throwable {
-    doPutAll(RegionShortcut.REPLICATE_PERSISTENT);
-  }
+  public void removeAllWaitsForBackupTest() throws Throwable {
+    region.put(1, 1);
+    region.put(2, 2);
 
-  @Test
-  public void testRemoveAllWithParReg() throws Throwable {
-    doRemoveAll(RegionShortcut.PARTITION_PERSISTENT);
+    List<Integer> keys = Arrays.asList(1, 2);
+    doActionAndVerifyWaitForBackup(() -> region.removeAll(keys));
+    assertTrue(region.isEmpty());
   }
 
   @Test
-  public void testRemoveAllWithReplicate() throws Throwable {
-    doRemoveAll(RegionShortcut.REPLICATE_PERSISTENT);
-  }
-
-  /**
-   * Test that a create waits for backup
-   * 
-   * @param shortcut The region shortcut to use to create the region
-   * @throws InterruptedException
-   */
-  private void doCreate(RegionShortcut shortcut, boolean useCreate) throws InterruptedException {
-    Region aRegion = createRegion(shortcut);
-    Runnable runnable = new Runnable() {
-      public void run() {
-        if (useCreate) {
-          aRegion.create(1, 1);
-        } else {
-          aRegion.put(1, 1);
-        }
-      }
-    };
-
-    verifyWaitForBackup(runnable);
-    assertTrue(aRegion.containsKey(1));
-    assertEquals(aRegion.get(1), 1);
-  }
-
-  /**
-   * Test that an update waits for backup
-   * 
-   * @param shortcut The region shortcut to use to create the region
-   * @throws InterruptedException
-   */
-  private void doUpdate(RegionShortcut shortcut) throws InterruptedException {
-    Region aRegion = createRegion(shortcut);
-    aRegion.put(1, 1);
-
-    Runnable runnable = new Runnable() {
-      public void run() {
-        aRegion.put(1, 2);
-      }
-    };
-
-    verifyWaitForBackup(runnable);
-    assertTrue(aRegion.containsKey(1));
-    assertEquals(aRegion.get(1), 2);
-  }
-
-  /**
-   * Test that an invalidate waits for backup
-   * 
-   * @param shortcut The region shortcut to use to create the region
-   * @throws InterruptedException
-   */
-  private void doInvalidate(RegionShortcut shortcut) throws InterruptedException {
-    Region aRegion = createRegion(shortcut);
-    aRegion.put(1, 1);
-
-    Runnable runnable = (new Runnable() {
-      public void run() {
-        aRegion.invalidate(1);
-      }
-    });
-
-    verifyWaitForBackup(runnable);
-    assertTrue(aRegion.containsKey(1));
-    assertNull(aRegion.get(1));
+  public void readActionsDoNotBlockDuringBackup() {
+    region.put(1, 1);
+    doReadActionsAndVerifyCompletion();
   }
 
-  /**
-   * Test that a destroy waits for backup
-   * 
-   * @param shortcut The region shortcut to use to create the region
-   * @throws InterruptedException
-   */
-  private void doDestroy(RegionShortcut shortcut) throws InterruptedException {
-    Region aRegion = createRegion(shortcut);
-    aRegion.put(1, 1);
-
-    Runnable runnable = new Runnable() {
-      public void run() {
-        aRegion.destroy(1);
-      }
-    };
-
-    verifyWaitForBackup(runnable);
-    assertFalse(aRegion.containsKey(1));
-  }
-
-  /**
-   * Test that a read op does NOT wait for backup
-   * 
-   * @param shortcut The region shortcut to use to create the region
-   * @throws InterruptedException
-   */
-  private void doRead(RegionShortcut shortcut, String op) throws Exception {
-    Region aRegion = createRegion(shortcut);
-    aRegion.put(1, 1);
-
-    Runnable runnable = new Runnable() {
-      public void run() {
-        switch (op) {
-          case "get": {
-            aRegion.get(1);
-            break;
-          }
-          case "containsKey": {
-            aRegion.containsKey(1);
-            break;
-          }
-          case "containsValue": {
-            aRegion.containsValue(1);
-            break;
-          }
-          case "containsValueForKey": {
-            aRegion.containsValue(1);
-            break;
-          }
-          case "entrySet": {
-            aRegion.entrySet();
-            break;
-          }
-          case "existsValue": {
-            try {
-              aRegion.existsValue("value = 1");
-            } catch (FunctionDomainException | TypeMismatchException | NameResolutionException
-                | QueryInvocationTargetException e) {
-              fail(e.toString());
-            }
-            break;
-          }
-          case "getAll": {
-            aRegion.getAll(new ArrayList());
-            break;
-          }
-          case "getEntry": {
-            aRegion.getEntry(1);
-            break;
-          }
-          case "isEmpty": {
-            aRegion.isEmpty();
-            break;
-          }
-          case "keySet": {
-            aRegion.keySet();
-            break;
-          }
-          case "query": {
-            try {
-              aRegion.query("select *");
-            } catch (FunctionDomainException | TypeMismatchException | NameResolutionException
-                | QueryInvocationTargetException e) {
-              fail(e.toString());
-            }
-            break;
-          }
-          case "size": {
-            aRegion.size();
-            break;
-          }
-          case "values": {
-            aRegion.values();
-            break;
-          }
-          default: {
-            fail("Unknown operation " + op);
-          }
-        }
-      }
-    };
-
-    verifyNoWaitForBackup(runnable);
-  }
-
-  /**
-   * Test that a putAll waits for backup
-   * 
-   * @param shortcut The region shortcut to use to create the region
-   * @throws InterruptedException
-   */
-  private void doPutAll(RegionShortcut shortcut) throws InterruptedException {
-    Region aRegion = createRegion(shortcut);
-    Runnable runnable = new Runnable() {
-      public void run() {
-        Map<Object, Object> putAllMap = new HashMap<Object, Object>();
-        putAllMap.put(1, 1);
-        putAllMap.put(2, 2);
-        aRegion.putAll(putAllMap);
-      }
-    };
-
-    verifyWaitForBackup(runnable);
-    assertTrue(aRegion.containsKey(1));
-    assertEquals(aRegion.get(1), 1);
-    assertTrue(aRegion.containsKey(2));
-    assertEquals(aRegion.get(2), 2);
-  }
-
-  /**
-   * Test that a removeAll waits for backup
-   * 
-   * @param shortcut The region shortcut to use to create the region
-   * @throws InterruptedException
-   */
-  private void doRemoveAll(RegionShortcut shortcut) throws InterruptedException {
-    Region aRegion = createRegion(shortcut);
-    aRegion.put(1, 2);
-    aRegion.put(2, 3);
-
-    Runnable runnable = new Runnable() {
-      public void run() {
-        List<Object> keys = new ArrayList();
-        keys.add(1);
-        keys.add(2);
-        aRegion.removeAll(keys);
-      }
-    };
-
-    verifyWaitForBackup(runnable);
-    assertEquals(aRegion.size(), 0);
+  private void doActionAndVerifyWaitForBackup(Runnable function)
+      throws InterruptedException, TimeoutException, ExecutionException {
+    DM dm = GemFireCacheImpl.getInstance().getDistributionManager();
+    Set recipients = dm.getOtherDistributionManagerIds();
+    Future<Void> future = null;
+    PrepareBackupRequest.send(dm, recipients);
+    waitingForBackupLockCount = 0;
+    future = CompletableFuture.runAsync(function);
+    Awaitility.await().atMost(5, TimeUnit.SECONDS)
+        .until(() -> assertTrue(waitingForBackupLockCount == 1));
+    FinishBackupRequest.send(dm, recipients, diskDirs[0], null, false);
+    future.get(5, TimeUnit.SECONDS);
   }
 
-  /**
-   * Test that executing the given runnable waits for backup completion to proceed
-   * 
-   * @param runnable The code that should wait for backup.
-   * @throws InterruptedException
-   */
-  private void verifyWaitForBackup(Runnable runnable) throws InterruptedException {
-    DM dm = ((InternalCache) GemFireCacheImpl.getInstance()).getDistributionManager();
+  private void doReadActionsAndVerifyCompletion() {
+    DM dm = GemFireCacheImpl.getInstance().getDistributionManager();
     Set recipients = dm.getOtherDistributionManagerIds();
-    boolean abort = true;
-    Thread aThread = new Thread(runnable);
+    PrepareBackupRequest.send(dm, recipients);
+    waitingForBackupLockCount = 0;
+    List<CompletableFuture<?>> futureList = doReadActions();
+    CompletableFuture.allOf(futureList.toArray(new CompletableFuture<?>[futureList.size()]));
+    assertTrue(waitingForBackupLockCount == 0);
+    FinishBackupRequest.send(dm, recipients, diskDirs[0], null, false);
+  }
+
+  private void verifyKeyValuePair(Integer key, Integer expectedValue) {
+    assertTrue(region.containsKey(key));
+    assertEquals(expectedValue, region.get(key));
+  }
+
+  private List<CompletableFuture<?>> doReadActions() {
+    List<Runnable> actions = new ArrayList<>();
+    actions.add(() -> region.get(1));
+    actions.add(() -> region.containsKey(1));
+    actions.add(() -> region.containsValue(1));
+    actions.add(region::entrySet);
+    actions.add(this::valueExistsCheck);
+    actions.add(() -> region.getAll(Collections.emptyList()));
+    actions.add(() -> region.getEntry(1));
+    actions.add(region::isEmpty);
+    actions.add(region::keySet);
+    actions.add(region::size);
+    actions.add(region::values);
+    actions.add(this::queryCheck);
+    return actions.stream().map(runnable -> CompletableFuture.runAsync(runnable))
+        .collect(Collectors.toList());
+  }
+
+  private void valueExistsCheck() {
     try {
-      PrepareBackupRequest.send(dm, recipients);
-      abort = false;
-      waitingForBackupLockCount = 0;
-      aThread.start();
-      Awaitility.await().atMost(30, TimeUnit.SECONDS)
-          .until(() -> assertTrue(waitingForBackupLockCount == 1));
-    } finally {
-      FinishBackupRequest.send(dm, recipients, diskDirs[0], null, abort);
-      aThread.join(30000);
-      assertFalse(aThread.isAlive());
+      region.existsValue("value = 1");
+    } catch (FunctionDomainException | TypeMismatchException | NameResolutionException
+        | QueryInvocationTargetException e) {
+      throw new RuntimeException(e);
     }
   }
 
-  /**
-   * Test that executing the given runnable does NOT wait for backup completion to proceed
-   * 
-   * @param runnable The code that should not wait for backup.
-   * @throws InterruptedException
-   */
-  private void verifyNoWaitForBackup(Runnable runnable) throws InterruptedException {
-    DM dm = ((InternalCache) GemFireCacheImpl.getInstance()).getDistributionManager();
-    Set recipients = dm.getOtherDistributionManagerIds();
-    boolean abort = true;
-    Thread aThread = new Thread(runnable);
+  private void queryCheck() {
     try {
-      PrepareBackupRequest.send(dm, recipients);
-      abort = false;
-      waitingForBackupLockCount = 0;
-      aThread.start();
-      aThread.join(30000);
-      assertFalse(aThread.isAlive());
-      assertTrue(waitingForBackupLockCount == 0);
-    } finally {
-      FinishBackupRequest.send(dm, recipients, diskDirs[0], null, abort);
+      region.query("select * from /" + TEST_REGION_NAME);
+    } catch (FunctionDomainException | TypeMismatchException | NameResolutionException
+        | QueryInvocationTargetException e) {
+      throw new RuntimeException(e);
     }
   }
 
@@ -549,7 +217,7 @@ public class BackupPrepareAndFinishMsgDUnitTest extends CacheTestCase {
    * @param shortcut The region shortcut to use to create the region
    * @return The newly created region.
    */
-  private Region<?, ?> createRegion(RegionShortcut shortcut) {
+  protected Region<Integer, Integer> createRegion(RegionShortcut shortcut) {
     Cache cache = getCache();
     DiskStoreFactory diskStoreFactory = cache.createDiskStoreFactory();
     diskDirs = getDiskDirs();
@@ -557,7 +225,7 @@ public class BackupPrepareAndFinishMsgDUnitTest extends CacheTestCase {
     DiskStore diskStore = diskStoreFactory.create(getUniqueName());
     ((DiskStoreImpl) diskStore).getBackupLock().setBackupLockTestHook(new BackupLockHook());
 
-    RegionFactory<String, String> regionFactory = cache.createRegionFactory(shortcut);
+    RegionFactory<Integer, Integer> regionFactory = cache.createRegionFactory(shortcut);
     regionFactory.setDiskStoreName(diskStore.getName());
     regionFactory.setDiskSynchronous(true);
     if (shortcut.equals(RegionShortcut.PARTITION_PERSISTENT)) {
@@ -565,7 +233,7 @@ public class BackupPrepareAndFinishMsgDUnitTest extends CacheTestCase {
       prFactory.setTotalNumBuckets(1);
       regionFactory.setPartitionAttributes(prFactory.create());
     }
-    return regionFactory.create("TestRegion");
+    return regionFactory.create(TEST_REGION_NAME);
   }
 
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/3bb6a221/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/PartitionedBackupPrepareAndFinishMsgDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/PartitionedBackupPrepareAndFinishMsgDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/PartitionedBackupPrepareAndFinishMsgDUnitTest.java
new file mode 100644
index 0000000..4b42c21
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/PartitionedBackupPrepareAndFinishMsgDUnitTest.java
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.persistence;
+
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+
+public class PartitionedBackupPrepareAndFinishMsgDUnitTest
+    extends BackupPrepareAndFinishMsgDUnitTest {
+  private static final RegionShortcut REGION_TYPE = RegionShortcut.PARTITION_PERSISTENT;
+
+  @Override
+  public Region<Integer, Integer> createRegion() {
+    return createRegion(REGION_TYPE);
+  }
+}

http://git-wip-us.apache.org/repos/asf/geode/blob/3bb6a221/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/ReplicateBackupPrepareAndFinishMsgDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/ReplicateBackupPrepareAndFinishMsgDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/ReplicateBackupPrepareAndFinishMsgDUnitTest.java
new file mode 100644
index 0000000..3f0ba7d
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/ReplicateBackupPrepareAndFinishMsgDUnitTest.java
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.persistence;
+
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+
+public class ReplicateBackupPrepareAndFinishMsgDUnitTest
+    extends BackupPrepareAndFinishMsgDUnitTest {
+  private static final RegionShortcut REGION_TYPE = RegionShortcut.REPLICATE_PERSISTENT;
+
+  @Override
+  public Region<Integer, Integer> createRegion() {
+    return createRegion(REGION_TYPE);
+  }
+}

http://git-wip-us.apache.org/repos/asf/geode/blob/3bb6a221/geode-core/src/test/java/org/apache/geode/management/internal/beans/DistributedSystemBridgeJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/beans/DistributedSystemBridgeJUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/beans/DistributedSystemBridgeJUnitTest.java
index bdf097e..60fb859 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/beans/DistributedSystemBridgeJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/beans/DistributedSystemBridgeJUnitTest.java
@@ -32,7 +32,7 @@ import org.apache.geode.admin.internal.PrepareBackupRequest;
 import org.apache.geode.distributed.internal.DM;
 import org.apache.geode.distributed.internal.locks.DLockService;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.persistence.BackupManager;
+import org.apache.geode.internal.cache.BackupManager;
 import org.apache.geode.internal.cache.persistence.PersistentMemberManager;
 import org.apache.geode.test.fake.Fakes;
 import org.apache.geode.test.junit.categories.UnitTest;
@@ -74,9 +74,9 @@ public class DistributedSystemBridgeJUnitTest {
 
     InOrder inOrder = inOrder(dm, backupManager);
     inOrder.verify(dm).putOutgoing(isA(PrepareBackupRequest.class));
-    inOrder.verify(backupManager).prepareBackup();
+    inOrder.verify(backupManager).prepareForBackup();
     inOrder.verify(dm).putOutgoing(isA(FinishBackupRequest.class));
-    inOrder.verify(backupManager).finishBackup(any(), any(), eq(false));
+    inOrder.verify(backupManager).doBackup(any(), any(), eq(false));
   }
 
   @Test
@@ -99,6 +99,6 @@ public class DistributedSystemBridgeJUnitTest {
     }
 
     verify(dm).putOutgoing(isA(FinishBackupRequest.class));
-    verify(backupManager).finishBackup(any(), any(), eq(true));
+    verify(backupManager).doBackup(any(), any(), eq(true));
   }
 }