geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From esh...@apache.org
Subject [geode] branch feature/GEODE-3521 updated: Change the system property name.
Date Fri, 13 Oct 2017 22:08:22 GMT
This is an automated email from the ASF dual-hosted git repository.

eshu11 pushed a commit to branch feature/GEODE-3521
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-3521 by this push:
     new 635a2f4  Change the system property name.
635a2f4 is described below

commit 635a2f4d6188eee6097a9f09099c457afef685fc
Author: eshu <eshu@pivotal.io>
AuthorDate: Fri Oct 13 15:03:08 2017 -0700

    Change the system property name.
    
      Make javadoc changes.
      Add the new tests specifical to the set operations with transaction.
---
 .../apache/geode/internal/cache/LocalRegion.java   |  10 +-
 .../geode/internal/cache/PartitionedRegion.java    |   6 +-
 .../apache/geode/internal/cache/TXManagerImpl.java |  42 ++--
 .../apache/geode/internal/cache/TXStateProxy.java  |   6 +-
 .../geode/internal/cache/TXStateProxyImpl.java     |   6 +-
 .../geode/internal/lang/SystemPropertyHelper.java  |  17 +-
 .../org/apache/geode/SetOperationTXJUnitTest.java  | 167 ++++++++++++++
 .../test/java/org/apache/geode/TX2JUnitTest.java   | 204 ----------------
 .../internal/cache/execute/PRJTADUnitTest.java     | 255 --------------------
 .../cache/execute/PRSetOperationJTADUnitTest.java  | 256 +++++++++++++++++++++
 .../cache/execute/PRSetOperationTXDUnitTest.java   | 237 +++++++++++++++++++
 .../cache/execute/PRTransaction2DUnitTest.java     | 204 +---------------
 .../internal/jta/JtaIntegrationJUnitTest.java      | 165 -------------
 .../internal/jta/SetOperationJTAJUnitTest.java     | 183 +++++++++++++++
 .../lang/SystemPropertyHelperJUnitTest.java        |  24 +-
 15 files changed, 913 insertions(+), 869 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 420ca4a..5d89053 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -1844,7 +1844,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
   public Set entrySet(boolean recursive) {
     checkReadiness();
     checkForNoAccess();
-    if (!preventSetOpBootstrapTransaction) {
+    if (!restoreSetOperationTransactionBehavior) {
       discoverJTA();
     }
     return basicEntries(recursive);
@@ -1869,7 +1869,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
   public Set keys() {
     checkReadiness();
     checkForNoAccess();
-    if (!preventSetOpBootstrapTransaction) {
+    if (!restoreSetOperationTransactionBehavior) {
       discoverJTA();
     }
     return new EntriesSet(this, false, IteratorType.KEYS, false);
@@ -1891,7 +1891,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
   public Collection values() {
     checkReadiness();
     checkForNoAccess();
-    if (!preventSetOpBootstrapTransaction) {
+    if (!restoreSetOperationTransactionBehavior) {
       discoverJTA();
     }
     return new EntriesSet(this, false, IteratorType.VALUES, false);
@@ -6401,8 +6401,8 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
     getDataView().destroyExistingEntry(event, cacheWrite, expectedOldValue);
   }
 
-  protected final boolean preventSetOpBootstrapTransaction =
-      SystemPropertyHelper.preventSetOpBootstrapTransaction();
+  protected final boolean restoreSetOperationTransactionBehavior =
+      SystemPropertyHelper.restoreSetOperationTransactionBehavior();
 
   /**
    * Do the expensive work of discovering an existing JTA transaction Only needs to be called at
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
index 35fcff6..be64751 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
@@ -5891,7 +5891,7 @@ public class PartitionedRegion extends LocalRegion
   @Override
   public Set entrySet(boolean recursive) {
     checkReadiness();
-    if (!preventSetOpBootstrapTransaction) {
+    if (!restoreSetOperationTransactionBehavior) {
       discoverJTA();
     }
     return Collections.unmodifiableSet(new PREntriesSet());
@@ -5957,7 +5957,7 @@ public class PartitionedRegion extends LocalRegion
   @Override
   public Set keys() {
     checkReadiness();
-    if (!preventSetOpBootstrapTransaction) {
+    if (!restoreSetOperationTransactionBehavior) {
       discoverJTA();
     }
     return Collections.unmodifiableSet(new KeysSet());
@@ -6146,7 +6146,7 @@ public class PartitionedRegion extends LocalRegion
   @Override
   public Collection values() {
     checkReadiness();
-    if (!preventSetOpBootstrapTransaction) {
+    if (!restoreSetOperationTransactionBehavior) {
       discoverJTA();
     }
     return Collections.unmodifiableSet(new ValuesSet());
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/TXManagerImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/TXManagerImpl.java
index ed5f921..9d35938 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/TXManagerImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/TXManagerImpl.java
@@ -590,7 +590,7 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
    */
   public TXStateProxy getTXState() {
     TXStateProxy tsp = txContext.get();
-    if (tsp == pausedTXState) {
+    if (tsp == PAUSED) {
       // treats paused transaction as no transaction.
       return null;
     }
@@ -612,7 +612,7 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
   public boolean setInProgress(boolean progress) {
     boolean retVal = false;
     TXStateProxy tsp = txContext.get();
-    assert tsp != pausedTXState;
+    assert tsp != PAUSED;
     if (tsp != null) {
       retVal = tsp.isInProgress();
       tsp.setInProgress(progress);
@@ -674,14 +674,15 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
     }
   }
 
-  private static final TXStateProxy pausedTXState = new PausedTXStateProxyImpl();
+  private static final TXStateProxy PAUSED = new PausedTXStateProxyImpl();
 
   /**
    * If the current thread is in a transaction then pause will cause it to no longer be in a
    * transaction. The same thread is expected to unpause/resume the transaction later.
    *
-   * @return the state of the transaction or null. Pass this value to {@link TXManagerImpl#resume}
-   *         to reactivate the suspended transaction.
+   * @return the state of the transaction or null. Pass this value to
+   *         {@link TXManagerImpl#unpauseTransaction} to reactivate the puased/suspended
+   *         transaction.
    */
   public TXStateProxy pauseTransaction() {
     return internalSuspend(true);
@@ -689,11 +690,13 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
 
   /**
    * If the current thread is in a transaction then suspend will cause it to no longer be in a
-   * transaction.
+   * transaction. Currently only used in testing.
    * 
-   * @return the state of the transaction or null. Pass this value to {@link TXManagerImpl#resume}
-   *         to reactivate the suspended transaction.
+   * @return the state of the transaction or null. to reactivate the suspended transaction.
+   * @deprecated use {@link TXManagerImpl#pauseTransaction} or
+   *             {@link CacheTransactionManager#suspend} instead
    */
+  @Deprecated
   public TXStateProxy internalSuspend() {
     return internalSuspend(false);
   }
@@ -704,15 +707,16 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
    *
    * @param needToResumeBySameThread whether a suspended transaction needs to be resumed by the same
    *        thread.
-   * @return the state of the transaction or null. Pass this value to {@link TXManagerImpl#resume}
-   *         to reactivate the suspended transaction.
+   * @return the state of the transaction or null. Pass this value to
+   *         {@link TXManagerImpl#internalResume(TXStateProxy, boolean)} to reactivate the suspended
+   *         transaction.
    */
-  public TXStateProxy internalSuspend(boolean needToResumeBySameThread) {
+  private TXStateProxy internalSuspend(boolean needToResumeBySameThread) {
     TXStateProxy result = getTXState();
     if (result != null) {
       result.suspend();
       if (needToResumeBySameThread) {
-        setTXState(pausedTXState);
+        setTXState(PAUSED);
       } else {
         setTXState(null);
       }
@@ -734,11 +738,15 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
 
   /**
    * Activates the specified transaction on the calling thread. Does not require the same thread to
-   * resume it.
+   * resume it. Currently only used in testing.
    *
    * @param tx the transaction to activate.
    * @throws IllegalStateException if this thread already has an active transaction
+   * 
+   * @deprecated use {@link TXManagerImpl#unpauseTransaction} or
+   *             {@link CacheTransactionManager#resume} instead
    */
+  @Deprecated
   public void internalResume(TXStateProxy tx) {
     internalResume(tx, false);
   }
@@ -751,7 +759,7 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
    *        thread.
    * @throws IllegalStateException if this thread already has an active transaction
    */
-  public void internalResume(TXStateProxy tx, boolean needToResumeBySameThread) {
+  private void internalResume(TXStateProxy tx, boolean needToResumeBySameThread) {
     if (tx != null) {
       TransactionId tid = getTransactionId();
       if (tid != null) {
@@ -761,7 +769,7 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
       }
       if (needToResumeBySameThread) {
         TXStateProxy result = txContext.get();
-        if (result != pausedTXState) {
+        if (result != PAUSED) {
           throw new java.lang.IllegalStateException(
               "try to unpause a transaction not paused by the same thread");
         }
@@ -773,7 +781,7 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
   }
 
   public boolean isTransactionPaused() {
-    return txContext.get() == pausedTXState;
+    return txContext.get() == PAUSED;
   }
 
   /**
@@ -821,7 +829,7 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
 
   public int getMyTXUniqueId() {
     TXStateProxy t = txContext.get();
-    if (t != null && t != pausedTXState) {
+    if (t != null && t != PAUSED) {
       return t.getTxId().getUniqId();
     } else {
       return NOTX;
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxy.java b/geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxy.java
index 355900e..c7ba38c 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxy.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxy.java
@@ -64,14 +64,12 @@ public interface TXStateProxy extends TXStateInterface {
   public TXSynchronizationRunnable getSynchronizationRunnable();
 
   /**
-   * Called by {@link TXManagerImpl#internalSuspend(boolean)} to perform additional tasks required
-   * to suspend a transaction
+   * Perform additional tasks required by the proxy to suspend a transaction
    */
   public void suspend();
 
   /**
-   * Called by {@link TXManagerImpl#internalResume(TXStateProxy, boolean)} to perform additional
-   * tasks required to resume a transaction
+   * Perform additional tasks required by the proxy to resume a transaction
    */
   public void resume();
 
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxyImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxyImpl.java
index d01b8d6..7a1e202 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxyImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxyImpl.java
@@ -509,8 +509,8 @@ public class TXStateProxyImpl implements TXStateProxy {
     return getRealDeal(null, currRgn).getAdditionalKeysForIterator(currRgn);
   }
 
-  protected final boolean preventSetOpBootstrapTransaction =
-      SystemPropertyHelper.preventSetOpBootstrapTransaction();
+  protected final boolean restoreSetOperationTransactionBehavior =
+      SystemPropertyHelper.restoreSetOperationTransactionBehavior();
 
   public Object getEntryForIterator(KeyInfo key, LocalRegion currRgn, boolean rememberReads,
       boolean allowTombstones) {
@@ -534,7 +534,7 @@ public class TXStateProxyImpl implements TXStateProxy {
 
   private boolean isTransactionInternalSuspendNeeded(LocalRegion region) {
     boolean resetTxState = this.realDeal == null
-        && (!region.canStoreDataLocally() || preventSetOpBootstrapTransaction);
+        && (!region.canStoreDataLocally() || restoreSetOperationTransactionBehavior);
     return resetTxState;
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/internal/lang/SystemPropertyHelper.java b/geode-core/src/main/java/org/apache/geode/internal/lang/SystemPropertyHelper.java
index 784da68..97d6343 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/lang/SystemPropertyHelper.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/lang/SystemPropertyHelper.java
@@ -18,7 +18,7 @@ package org.apache.geode.internal.lang;
  * The SystemPropertyHelper class is an helper class for accessing system properties used in geode.
  * The method name to get the system property should be the same as the system property name.
  * 
- * @since Geode 1.3.0
+ * @since Geode 1.4.0
  */
 
 public class SystemPropertyHelper {
@@ -41,12 +41,17 @@ public class SystemPropertyHelper {
   }
 
   /**
-   * A set operation can bootstrap a transaction now. User need to specifically disable this by
-   * setting this system property to true to get the old behavior.
+   * As of Geode 1.4.0, a region set operation will be in a transaction even if it is the first
+   * operation in the transaction.
    *
-   * @since Geode 1.3.0
+   * In previous releases, a region operation is not in a transaction if it is the first operation
+   * of the transaction.
+   *
+   * Setting this system property to true will restore the previous behavior.
+   *
+   * @since Geode 1.4.0
    */
-  public static boolean preventSetOpBootstrapTransaction() {
-    return getProductBooleanProperty("preventSetOpBootstrapTransaction");
+  public static boolean restoreSetOperationTransactionBehavior() {
+    return getProductBooleanProperty("restoreSetOperationTransactionBehavior");
   }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/SetOperationTXJUnitTest.java b/geode-core/src/test/java/org/apache/geode/SetOperationTXJUnitTest.java
new file mode 100644
index 0000000..78cf324
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/SetOperationTXJUnitTest.java
@@ -0,0 +1,167 @@
+/*
+ * 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;
+
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.junit.Assert.*;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.CacheTransactionManager;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.cache.TXManagerImpl;
+import org.apache.geode.internal.cache.TXStateProxyImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.logging.log4j.Logger;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+
+@Category(IntegrationTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class SetOperationTXJUnitTest {
+
+  private static final Logger logger = LogService.getLogger();
+  private static final String REGION_NAME = "region1";
+
+  private Map<Long, String> testData;
+  private Cache cache;
+
+  @Rule
+  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
+
+  @Before
+  public void setup() {
+    testData = new HashMap<>();
+    testData.put(1L, "value1");
+    testData.put(2L, "value2");
+    testData.put(3L, "duplicateValue");
+    testData.put(4L, "duplicateValue");
+  }
+
+  @After
+  public void tearDownTest() throws Exception {
+    closeCache();
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionKeysetWithTx(boolean disableSetOpToStartTx) {
+    Region<Long, String> region = setupAndLoadRegion(disableSetOpToStartTx);
+    CacheTransactionManager txMgr = cache.getCacheTransactionManager();
+    try {
+      txMgr.begin();
+      Collection<Long> set = region.keySet();
+      set.forEach((key) -> assertTrue(testData.keySet().contains(key)));
+    } finally {
+      validateTXManager(disableSetOpToStartTx);
+      txMgr.rollback();
+    }
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionValuesWithTx(boolean disableSetOpToStartTx) {
+    Region<Long, String> region = setupAndLoadRegion(disableSetOpToStartTx);
+    CacheTransactionManager txMgr = cache.getCacheTransactionManager();
+    try {
+      txMgr.begin();
+      Collection<String> set = region.values();
+      set.forEach((value) -> assertTrue(testData.values().contains(value)));
+    } finally {
+      validateTXManager(disableSetOpToStartTx);
+      txMgr.rollback();
+    }
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionEntriesWithTx(boolean disableSetOpToStartTx) {
+    Region<Long, String> region = setupAndLoadRegion(disableSetOpToStartTx);
+    CacheTransactionManager txMgr = cache.getCacheTransactionManager();
+    try {
+      txMgr.begin();
+      Collection<Map.Entry<Long, String>> set = region.entrySet();
+      set.forEach((entry) -> {
+        assertTrue(testData.values().contains(entry.getValue()));
+        assertTrue(testData.keySet().contains(entry.getKey()));
+      });
+    } finally {
+      validateTXManager(disableSetOpToStartTx);
+      txMgr.rollback();
+    }
+  }
+
+  private Region<Long, String> setupAndLoadRegion(boolean disableSetOpToStartTx) {
+    this.cache = createCache(disableSetOpToStartTx);
+    Region<Long, String> region = createRegion(cache);
+    testData.forEach((k, v) -> region.put(k, v));
+    return region;
+  }
+
+  private void validateTXManager(boolean disableSetOpToStartTx) {
+    assertNotNull(TXManagerImpl.getCurrentTXState());
+    if (disableSetOpToStartTx) {
+      assertFalse(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
+    } else {
+      assertTrue(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
+    }
+  }
+
+  protected Region<Long, String> createRegion(Cache cache) {
+    RegionFactory<Long, String> rf = cache.createRegionFactory(RegionShortcut.REPLICATE);
+    Region<Long, String> r = rf.create(REGION_NAME);
+    return r;
+  }
+
+  final String restoreSetOperationTransactionBehavior = "restoreSetOperationTransactionBehavior";
+  final String RESTORE_SET_OPERATION_PROPERTY =
+      (System.currentTimeMillis() % 2 == 0 ? DistributionConfig.GEMFIRE_PREFIX : "geode.")
+          + restoreSetOperationTransactionBehavior;
+
+  private Cache createCache(boolean disableSetOpToStartTx) {
+    if (disableSetOpToStartTx) {
+      logger.info("setting system property {} to true ", RESTORE_SET_OPERATION_PROPERTY);
+      System.setProperty(RESTORE_SET_OPERATION_PROPERTY, "true");
+    }
+    CacheFactory cf = new CacheFactory().set(MCAST_PORT, "0");
+    this.cache = (GemFireCacheImpl) cf.create();
+    return this.cache;
+  }
+
+  protected void closeCache() {
+    if (this.cache != null) {
+      Cache c = this.cache;
+      this.cache = null;
+      c.close();
+    }
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/TX2JUnitTest.java b/geode-core/src/test/java/org/apache/geode/TX2JUnitTest.java
deleted file mode 100644
index 8139652..0000000
--- a/geode-core/src/test/java/org/apache/geode/TX2JUnitTest.java
+++ /dev/null
@@ -1,204 +0,0 @@
-/*
- * 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;
-
-import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
-import static org.junit.Assert.*;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-
-import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.CacheFactory;
-import org.apache.geode.cache.CacheTransactionManager;
-import org.apache.geode.cache.Region;
-import org.apache.geode.cache.RegionFactory;
-import org.apache.geode.cache.RegionShortcut;
-import org.apache.geode.distributed.internal.DistributionConfig;
-import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.TXManagerImpl;
-import org.apache.geode.internal.cache.TXStateProxyImpl;
-import org.apache.geode.internal.logging.LogService;
-import org.apache.geode.test.junit.categories.IntegrationTest;
-import org.apache.logging.log4j.Logger;
-import org.junit.After;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.contrib.java.lang.system.RestoreSystemProperties;
-import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
-
-import junitparams.JUnitParamsRunner;
-import junitparams.Parameters;
-
-@Category(IntegrationTest.class)
-@RunWith(JUnitParamsRunner.class)
-public class TX2JUnitTest {
-
-  private static final Logger logger = LogService.getLogger();
-
-  @Rule
-  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
-
-  long k1 = 1;
-  long k2 = 2;
-  long k3 = 3;
-  long k4 = 4;
-  String v1 = "value1";
-  String v2 = "value2";
-  String v3 = "value3";
-  String regionName = "region1";
-  Cache cache;
-
-  enum SetOp {
-    KEYSET, VALUES, ENTRYSET;
-  }
-
-  @After
-  public void tearDownTXJUnitTest() throws Exception {
-    closeCache();
-  }
-
-  @Test
-  @Parameters
-  public void testRegionSetOpWithTx(SetOp op, boolean preventSetOpToStartTx) {
-    verifySetOp(SetOp.VALUES, false);
-  }
-
-  private Object[] parametersForTestRegionSetOpWithTx() {
-    return new Object[] {new Object[] {SetOp.VALUES, false}, new Object[] {SetOp.VALUES, true},
-        new Object[] {SetOp.KEYSET, false}, new Object[] {SetOp.KEYSET, true},
-        new Object[] {SetOp.ENTRYSET, false}, new Object[] {SetOp.ENTRYSET, true},};
-  }
-
-  private void verifySetOp(SetOp op, boolean preventSetOpToStartTx) {
-    this.cache = createCache(preventSetOpToStartTx);
-    Region<Long, String> region = createRegion(cache);
-    initRegion(region);
-
-    basicVerifySetOp(op, preventSetOpToStartTx, cache);
-  }
-
-  private void basicVerifySetOp(SetOp op, boolean preventSetOpToStartTx, Cache cache) {
-    Region<Long, String> region = cache.getRegion(Region.SEPARATOR + regionName);
-
-    Collection<Long> keys = new ArrayList<Long>();
-    keys.add(k1);
-    keys.add(k2);
-    keys.add(k3);
-    keys.add(k4);
-    Collection<String> values = new ArrayList<String>();
-    values.add(v1);
-    values.add(v2);
-    values.add(v3);
-    CacheTransactionManager txMgr = cache.getCacheTransactionManager();
-
-    try {
-      txMgr.begin();
-
-      @SuppressWarnings("rawtypes")
-      Collection set = getSetOp(region, op);
-
-      verifySetOp(op, region, keys, values, set);
-    } finally {
-      assertNotNull(TXManagerImpl.getCurrentTXState());
-      if (preventSetOpToStartTx) {
-        assertFalse(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
-      } else {
-        assertTrue(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
-      }
-      txMgr.rollback();
-    }
-  }
-
-  protected Region<Long, String> createRegion(Cache cache) {
-    RegionFactory<Long, String> rf = cache.createRegionFactory(RegionShortcut.REPLICATE);
-    Region<Long, String> r = rf.create(regionName);
-    return r;
-  }
-
-  final String preventSetOpBootstrapTransaction = "preventSetOpBootstrapTransaction";
-  final String PREVENT_SET_OP_BOOTSTRAP_TRANSACTION =
-      (System.currentTimeMillis() % 2 == 0 ? DistributionConfig.GEMFIRE_PREFIX : "geode.")
-          + preventSetOpBootstrapTransaction;
-
-  private Cache createCache(boolean preventSetOpToStartTx) {
-    if (preventSetOpToStartTx) {
-      logger.info("setting system property {} to true ", PREVENT_SET_OP_BOOTSTRAP_TRANSACTION);
-      System.setProperty(PREVENT_SET_OP_BOOTSTRAP_TRANSACTION, "true");
-    }
-    CacheFactory cf = new CacheFactory().set(MCAST_PORT, "0");
-    this.cache = (GemFireCacheImpl) cf.create();
-    return this.cache;
-  }
-
-  protected void closeCache() {
-    if (this.cache != null) {
-      Cache c = this.cache;
-      this.cache = null;
-      c.close();
-    }
-  }
-
-  private void initRegion(Region<Long, String> region) {
-    region.put(k1, v1);
-    region.put(k2, v2);
-    region.put(k3, v3);
-    region.put(k4, v3);
-  }
-
-  @SuppressWarnings("rawtypes")
-  private Collection getSetOp(Region<Long, String> region, SetOp op) {
-    Collection set = null;
-    switch (op) {
-      case VALUES:
-        set = region.values();
-        break;
-      case KEYSET:
-        set = region.keySet();
-        break;
-      case ENTRYSET:
-        set = region.entrySet();
-        break;
-      default:
-        fail("Unexpected op: " + op);
-    }
-    return set;
-  }
-
-  @SuppressWarnings("rawtypes")
-  private void verifySetOp(SetOp op, Region<Long, String> region, Collection<Long> keys,
-      Collection<String> values, Collection set) {
-    Iterator it = set.iterator();
-    while (it.hasNext()) {
-      Object o = it.next();
-      switch (op) {
-        case VALUES:
-          assertTrue(values.contains(o));
-          break;
-        case KEYSET:
-          assertTrue(keys.contains(o));
-          break;
-        case ENTRYSET:
-          assertTrue(keys.contains(((Region.Entry) o).getKey()));
-          assertTrue(values.contains(((Region.Entry) o).getValue()));
-          break;
-        default:
-          fail("Unexpected op: " + op);
-      }
-    }
-  }
-}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRJTADUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRJTADUnitTest.java
deleted file mode 100644
index 35beec5..0000000
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRJTADUnitTest.java
+++ /dev/null
@@ -1,255 +0,0 @@
-/*
- * 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.execute;
-
-import static org.junit.Assert.*;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-
-import javax.naming.Context;
-import javax.naming.NamingException;
-import javax.transaction.NotSupportedException;
-import javax.transaction.SystemException;
-import javax.transaction.UserTransaction;
-
-import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.PartitionAttributesFactory;
-import org.apache.geode.cache.Region;
-import org.apache.geode.cache.RegionShortcut;
-import org.apache.geode.distributed.internal.DistributionConfig;
-import org.apache.geode.internal.cache.TXManagerImpl;
-import org.apache.geode.internal.logging.LogService;
-import org.apache.geode.test.dunit.Host;
-import org.apache.geode.test.dunit.Invoke;
-import org.apache.geode.test.dunit.VM;
-import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
-import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
-import org.apache.geode.test.junit.categories.DistributedTest;
-import org.apache.logging.log4j.Logger;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
-
-import junitparams.JUnitParamsRunner;
-import junitparams.Parameters;
-
-/**
- * Test for JTA with PR.
- */
-@Category(DistributedTest.class)
-@RunWith(JUnitParamsRunner.class)
-@SuppressWarnings("serial")
-public class PRJTADUnitTest extends JUnit4CacheTestCase {
-
-  private static final Logger logger = LogService.getLogger();
-
-  @Rule
-  public DistributedRestoreSystemProperties restoreSystemProperties =
-      new DistributedRestoreSystemProperties();
-
-  public PRJTADUnitTest() {
-    super();
-  }
-
-  VM accessor = null;
-  VM dataStore1 = null;
-  VM dataStore2 = null;
-  VM dataStore3 = null;
-
-
-  @Override
-  public final void postSetUp() throws Exception {
-    disconnectAllFromDS(); // isolate this test from others to avoid periodic CacheExistsExceptions
-    Host host = Host.getHost(0);
-    dataStore1 = host.getVM(0);
-    dataStore2 = host.getVM(1);
-    dataStore3 = host.getVM(2);
-    accessor = host.getVM(3);
-  }
-
-  long k1 = 1;
-  long k2 = 2;
-  long k3 = 3;
-  long k4 = 4;
-  String v1 = "value1";
-  String v2 = "value2";
-  String v3 = "value3";
-
-  enum SetOp {
-    KEYSET, VALUES, ENTRYSET;
-  }
-
-  protected void createRegion(boolean preventSetOpToStartJTA) {
-    accessor.invoke(() -> createCache(preventSetOpToStartJTA));
-    dataStore1.invoke(() -> createCache(preventSetOpToStartJTA));
-    dataStore2.invoke(() -> createCache(preventSetOpToStartJTA));
-    dataStore3.invoke(() -> createCache(preventSetOpToStartJTA));
-
-    accessor.invoke(() -> createPR(true));
-    dataStore1.invoke(() -> createPR(false));
-    dataStore2.invoke(() -> createPR(false));
-    dataStore3.invoke(() -> createPR(false));
-  }
-
-  @Test
-  @Parameters
-  public void testJTAWithRegionSetOperation(final SetOp op, final boolean preventSetOpToStartJTA) {
-    verifyJTASetOp(op, preventSetOpToStartJTA);
-  }
-
-  private Object[] parametersForTestJTAWithRegionSetOperation() {
-    return new Object[] {new Object[] {SetOp.VALUES, false}, new Object[] {SetOp.VALUES, true},
-        new Object[] {SetOp.KEYSET, false}, new Object[] {SetOp.KEYSET, true},
-        new Object[] {SetOp.ENTRYSET, false}, new Object[] {SetOp.ENTRYSET, true},};
-  }
-
-  String regionName = "region1";
-
-  private void basicVerifySetOp(SetOp op, boolean preventSetOpToStartJTA, boolean isAccessor)
-      throws NotSupportedException, SystemException, NamingException {
-    Cache cache = basicGetCache();
-    Region<Long, String> region = cache.getRegion(Region.SEPARATOR + regionName);
-
-    Context ctx = cache.getJNDIContext();
-    UserTransaction ta = startUserTransaction(ctx);
-
-    Collection<Long> keys = new ArrayList<Long>();
-    keys.add(k1);
-    keys.add(k2);
-    keys.add(k3);
-    keys.add(k4);
-    Collection<String> values = new ArrayList<String>();
-    values.add(v1);
-    values.add(v2);
-    values.add(v3);
-
-    // Begin the user transaction
-    try {
-      ta.begin();
-
-      @SuppressWarnings("rawtypes")
-      Collection set = getSetOp(region, op);
-
-      verifyJTASetOp(op, region, keys, values, set);
-    } finally {
-      if (preventSetOpToStartJTA) {
-        assertNull(TXManagerImpl.getCurrentTXState());
-      } else {
-        assertNotNull(TXManagerImpl.getCurrentTXState());
-        if (!isAccessor) {
-          ta.rollback();
-        }
-      }
-    }
-  }
-
-  private void verifyJTASetOp(SetOp op, boolean preventSetOpToStartJTA) {
-    createRegion(preventSetOpToStartJTA);
-    dataStore1.invoke(() -> initRegion());
-
-    accessor.invoke(() -> basicVerifySetOp(op, preventSetOpToStartJTA, true));
-    dataStore1.invoke(() -> basicVerifySetOp(op, preventSetOpToStartJTA, false));
-    dataStore2.invoke(() -> basicVerifySetOp(op, preventSetOpToStartJTA, false));
-    dataStore3.invoke(() -> basicVerifySetOp(op, preventSetOpToStartJTA, false));
-  }
-
-  @SuppressWarnings("rawtypes")
-  private void verifyJTASetOp(SetOp op, Region<Long, String> region, Collection<Long> keys,
-      Collection<String> values, Collection set) {
-    Iterator it = set.iterator();
-    while (it.hasNext()) {
-      Object o = it.next();
-      switch (op) {
-        case VALUES:
-          assertTrue(values.contains(o));
-          break;
-        case KEYSET:
-          assertTrue(keys.contains(o));
-          break;
-        case ENTRYSET:
-          assertTrue(keys.contains(((Region.Entry) o).getKey()));
-          assertTrue(values.contains(((Region.Entry) o).getValue()));
-          break;
-        default:
-          fail("Unexpected op: " + op);
-      }
-    }
-  }
-
-  @SuppressWarnings("rawtypes")
-  private Collection getSetOp(Region<Long, String> region, SetOp op) {
-    Collection set = null;
-    switch (op) {
-      case VALUES:
-        set = region.values();
-        break;
-      case KEYSET:
-        set = region.keySet();
-        break;
-      case ENTRYSET:
-        set = region.entrySet();
-        break;
-      default:
-        fail("Unexpected op: " + op);
-    }
-    return set;
-  }
-
-  private void initRegion() {
-    Region<Long, String> currRegion;
-    currRegion = basicGetCache().getRegion(Region.SEPARATOR + regionName);
-    currRegion.put(k1, v1);
-    currRegion.put(k2, v2);
-    currRegion.put(k3, v3);
-    currRegion.put(k4, v3);
-  }
-
-  private UserTransaction startUserTransaction(Context ctx) throws NamingException {
-    return (UserTransaction) ctx.lookup("java:/UserTransaction");
-  }
-
-  @Override
-  public final void preTearDownCacheTestCase() throws Exception {
-    Invoke.invokeInEveryVM(() -> verifyNoTxState());
-  }
-
-  private void verifyNoTxState() {
-    TXManagerImpl mgr = getCache().getTxManager();
-    assertEquals(0, mgr.hostedTransactionsInProgressForTest());
-  }
-
-  final String preventSetOpBootstrapTransaction = "preventSetOpBootstrapTransaction";
-  final String PREVENT_SET_OP_BOOTSTRAP_TRANSACTION =
-      (System.currentTimeMillis() % 2 == 0 ? DistributionConfig.GEMFIRE_PREFIX : "geode.")
-          + preventSetOpBootstrapTransaction;
-
-  private void createCache(boolean preventSetOpToStartJTA) {
-    if (preventSetOpToStartJTA) {
-      logger.info("setting system property {} to true ", PREVENT_SET_OP_BOOTSTRAP_TRANSACTION);
-      System.setProperty(PREVENT_SET_OP_BOOTSTRAP_TRANSACTION, "true");
-    }
-    getCache();
-  }
-
-  private void createPR(boolean isAccessor) {
-    basicGetCache().createRegionFactory(RegionShortcut.PARTITION)
-        .setPartitionAttributes(new PartitionAttributesFactory<Long, String>().setTotalNumBuckets(3)
-            .setLocalMaxMemory(isAccessor ? 0 : 1).create())
-        .create(regionName);
-  }
-}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRSetOperationJTADUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRSetOperationJTADUnitTest.java
new file mode 100644
index 0000000..b5522e9
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRSetOperationJTADUnitTest.java
@@ -0,0 +1,256 @@
+/*
+ * 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.execute;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.naming.Context;
+import javax.transaction.UserTransaction;
+
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.cache.TXManagerImpl;
+import org.apache.geode.internal.cache.TXStateProxyImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.Invoke;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.logging.log4j.Logger;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+
+@Category(DistributedTest.class)
+@RunWith(JUnitParamsRunner.class)
+@SuppressWarnings("serial")
+public class PRSetOperationJTADUnitTest extends JUnit4CacheTestCase {
+
+  private static final Logger logger = LogService.getLogger();
+  private static final String REGION_NAME = "region1";
+
+  private Map<Long, String> testData;
+
+  private VM accessor = null;
+  private VM dataStore1 = null;
+  private VM dataStore2 = null;
+  private VM dataStore3 = null;
+
+  @Rule
+  public DistributedRestoreSystemProperties restoreSystemProperties =
+      new DistributedRestoreSystemProperties();
+
+  public PRSetOperationJTADUnitTest() {
+    super();
+  }
+
+  @Before
+  public void setup() {
+    testData = new HashMap<>();
+    testData.put(1L, "value1");
+    testData.put(2L, "value2");
+    testData.put(3L, "duplicateValue");
+    testData.put(4L, "duplicateValue");
+  }
+
+  @Override
+  public final void postSetUp() throws Exception {
+    disconnectAllFromDS(); // isolate this test from others to avoid periodic CacheExistsExceptions
+    Host host = Host.getHost(0);
+    dataStore1 = host.getVM(0);
+    dataStore2 = host.getVM(1);
+    dataStore3 = host.getVM(2);
+    accessor = host.getVM(3);
+  }
+
+  @Override
+  public final void preTearDownCacheTestCase() throws Exception {
+    Invoke.invokeInEveryVM(() -> verifyNoTxState());
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionKeysetWithJTA(boolean disableSetOpToStartJTA) throws Exception {
+    setupAndLoadRegion(disableSetOpToStartJTA);
+    verifyRegionKeysetWithJTA(disableSetOpToStartJTA);
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionValuesWithJTA(boolean disableSetOpToStartJTA) throws Exception {
+    setupAndLoadRegion(disableSetOpToStartJTA);
+    verifyRegionValuesWithJTA(disableSetOpToStartJTA);
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionEntriesWithJTA(boolean disableSetOpToStartJTA) throws Exception {
+    setupAndLoadRegion(disableSetOpToStartJTA);
+    verifyRegionEntriesWithJTA(disableSetOpToStartJTA);
+  }
+
+  private void setupAndLoadRegion(boolean disableSetOpToStartJTA) {
+    createRegion(disableSetOpToStartJTA);
+    dataStore1.invoke(() -> loadRegion());
+  }
+
+  private void createRegion(boolean disableSetOpToStartJTA) {
+    accessor.invoke(() -> createCache(disableSetOpToStartJTA));
+    dataStore1.invoke(() -> createCache(disableSetOpToStartJTA));
+    dataStore2.invoke(() -> createCache(disableSetOpToStartJTA));
+    dataStore3.invoke(() -> createCache(disableSetOpToStartJTA));
+
+    accessor.invoke(() -> createPR(true));
+    dataStore1.invoke(() -> createPR(false));
+    dataStore2.invoke(() -> createPR(false));
+    dataStore3.invoke(() -> createPR(false));
+  }
+
+  private void loadRegion() {
+    Region<Long, String> region = basicGetCache().getRegion(Region.SEPARATOR + REGION_NAME);
+    testData.forEach((k, v) -> region.put(k, v));
+  }
+
+  private void verifyRegionKeysetWithJTA(boolean disableSetOpToStartJTA) {
+    accessor.invoke(() -> verifyRegionKeysetWithJTA(disableSetOpToStartJTA, true));
+    dataStore1.invoke(() -> verifyRegionKeysetWithJTA(disableSetOpToStartJTA, false));
+    dataStore2.invoke(() -> verifyRegionKeysetWithJTA(disableSetOpToStartJTA, false));
+    dataStore3.invoke(() -> verifyRegionKeysetWithJTA(disableSetOpToStartJTA, false));
+  }
+
+  private void verifyRegionValuesWithJTA(boolean disableSetOpToStartJTA) {
+    accessor.invoke(() -> verifyRegionValuesWithJTA(disableSetOpToStartJTA, true));
+    dataStore1.invoke(() -> verifyRegionValuesWithJTA(disableSetOpToStartJTA, false));
+    dataStore2.invoke(() -> verifyRegionValuesWithJTA(disableSetOpToStartJTA, false));
+    dataStore3.invoke(() -> verifyRegionValuesWithJTA(disableSetOpToStartJTA, false));
+  }
+
+  private void verifyRegionEntriesWithJTA(boolean disableSetOpToStartJTA) {
+    accessor.invoke(() -> verifyRegionEntriesWithJTA(disableSetOpToStartJTA, true));
+    dataStore1.invoke(() -> verifyRegionEntriesWithJTA(disableSetOpToStartJTA, false));
+    dataStore2.invoke(() -> verifyRegionEntriesWithJTA(disableSetOpToStartJTA, false));
+    dataStore3.invoke(() -> verifyRegionEntriesWithJTA(disableSetOpToStartJTA, false));
+  }
+
+  private void verifyRegionKeysetWithJTA(boolean disableSetOpToStartJTA, boolean isAccessor)
+      throws Exception {
+    Context ctx = basicGetCache().getJNDIContext();
+    UserTransaction userTX = startUserTransaction(ctx);
+    Region<Long, String> region = basicGetCache().getRegion(Region.SEPARATOR + REGION_NAME);
+    try {
+      userTX.begin();
+      Collection<Long> set = region.keySet();
+      set.forEach((key) -> assertTrue(testData.keySet().contains(key)));
+    } finally {
+      validateTXManager(disableSetOpToStartJTA, isAccessor);
+      if (!disableSetOpToStartJTA && !isAccessor) {
+        userTX.rollback();
+      }
+    }
+  }
+
+  private void verifyRegionValuesWithJTA(boolean disableSetOpToStartJTA, boolean isAccessor)
+      throws Exception {
+    Context ctx = basicGetCache().getJNDIContext();
+    UserTransaction userTX = startUserTransaction(ctx);
+    Region<Long, String> region = basicGetCache().getRegion(Region.SEPARATOR + REGION_NAME);
+    try {
+      userTX.begin();
+      Collection<String> set = region.values();
+      set.forEach((value) -> assertTrue(testData.values().contains(value)));
+    } finally {
+      validateTXManager(disableSetOpToStartJTA, isAccessor);
+      if (!disableSetOpToStartJTA && !isAccessor) {
+        userTX.rollback();
+      }
+    }
+  }
+
+  private void verifyRegionEntriesWithJTA(boolean disableSetOpToStartJTA, boolean isAccessor)
+      throws Exception {
+    Context ctx = basicGetCache().getJNDIContext();
+    UserTransaction userTX = startUserTransaction(ctx);
+    Region<Long, String> region = basicGetCache().getRegion(Region.SEPARATOR + REGION_NAME);
+    try {
+      userTX.begin();
+      Collection<Map.Entry<Long, String>> set = region.entrySet();
+      set.forEach((entry) -> {
+        assertTrue(testData.values().contains(entry.getValue()));
+        assertTrue(testData.keySet().contains(entry.getKey()));
+      });
+    } finally {
+      validateTXManager(disableSetOpToStartJTA, isAccessor);
+      if (!disableSetOpToStartJTA && !isAccessor) {
+        userTX.rollback();
+      }
+    }
+  }
+
+  private void validateTXManager(boolean disableSetOpToStartTx, boolean isAccessor) {
+    if (disableSetOpToStartTx) {
+      assertNull(TXManagerImpl.getCurrentTXState());
+    } else {
+      assertNotNull(TXManagerImpl.getCurrentTXState());
+      if (!isAccessor) {
+        assertTrue(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
+      }
+    }
+  }
+
+  private UserTransaction startUserTransaction(Context ctx) throws Exception {
+    return (UserTransaction) ctx.lookup("java:/UserTransaction");
+  }
+
+  private void verifyNoTxState() {
+    TXManagerImpl mgr = getCache().getTxManager();
+    assertEquals(0, mgr.hostedTransactionsInProgressForTest());
+  }
+
+  final String restoreSetOperationTransactionBehavior = "restoreSetOperationTransactionBehavior";
+  final String RESTORE_SET_OPERATION_PROPERTY =
+      (System.currentTimeMillis() % 2 == 0 ? DistributionConfig.GEMFIRE_PREFIX : "geode.")
+          + restoreSetOperationTransactionBehavior;
+
+  private void createCache(boolean disableSetOpToStartJTA) {
+    if (disableSetOpToStartJTA) {
+      logger.info("setting system property {} to true ", RESTORE_SET_OPERATION_PROPERTY);
+      System.setProperty(RESTORE_SET_OPERATION_PROPERTY, "true");
+    }
+    getCache();
+  }
+
+  private void createPR(boolean isAccessor) {
+    basicGetCache().createRegionFactory(RegionShortcut.PARTITION)
+        .setPartitionAttributes(new PartitionAttributesFactory<Long, String>().setTotalNumBuckets(3)
+            .setLocalMaxMemory(isAccessor ? 0 : 1).create())
+        .create(REGION_NAME);
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRSetOperationTXDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRSetOperationTXDUnitTest.java
new file mode 100644
index 0000000..791f3fe
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRSetOperationTXDUnitTest.java
@@ -0,0 +1,237 @@
+/*
+ * 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.execute;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.geode.cache.CacheTransactionManager;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.cache.TXManagerImpl;
+import org.apache.geode.internal.cache.TXStateProxyImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.Invoke;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.logging.log4j.Logger;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+
+@Category(DistributedTest.class)
+@RunWith(JUnitParamsRunner.class)
+@SuppressWarnings("serial")
+public class PRSetOperationTXDUnitTest extends JUnit4CacheTestCase {
+
+  private static final Logger logger = LogService.getLogger();
+  private static final String REGION_NAME = "region1";
+
+  private Map<Long, String> testData;
+
+  private VM accessor = null;
+  private VM dataStore1 = null;
+  private VM dataStore2 = null;
+  private VM dataStore3 = null;
+
+  @Rule
+  public DistributedRestoreSystemProperties restoreSystemProperties =
+      new DistributedRestoreSystemProperties();
+
+  public PRSetOperationTXDUnitTest() {
+    super();
+  }
+
+  @Before
+  public void setup() {
+    testData = new HashMap<>();
+    testData.put(1L, "value1");
+    testData.put(2L, "value2");
+    testData.put(3L, "duplicateValue");
+    testData.put(4L, "duplicateValue");
+  }
+
+  @Override
+  public final void postSetUp() throws Exception {
+    disconnectAllFromDS(); // isolate this test from others to avoid periodic CacheExistsExceptions
+    Host host = Host.getHost(0);
+    dataStore1 = host.getVM(0);
+    dataStore2 = host.getVM(1);
+    dataStore3 = host.getVM(2);
+    accessor = host.getVM(3);
+  }
+
+  @Override
+  public final void preTearDownCacheTestCase() throws Exception {
+    Invoke.invokeInEveryVM(() -> verifyNoTxState());
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionKeysetWithTx(boolean disableSetOpToStartTx) throws Exception {
+    setupAndLoadRegion(disableSetOpToStartTx);
+    verifyRegionKeysetWithTx(disableSetOpToStartTx);
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionValuesWithTx(boolean disableSetOpToStartTx) throws Exception {
+    setupAndLoadRegion(disableSetOpToStartTx);
+    verifyRegionValuesWithTx(disableSetOpToStartTx);
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionEntriesWithTx(boolean disableSetOpToStartTx) throws Exception {
+    setupAndLoadRegion(disableSetOpToStartTx);
+    verifyRegionEntriesWithTx(disableSetOpToStartTx);
+  }
+
+  private void setupAndLoadRegion(boolean disableSetOpToStartTx) {
+    createRegion(disableSetOpToStartTx);
+    dataStore1.invoke(() -> loadRegion());
+  }
+
+  private void createRegion(boolean disableSetOpToStartTx) {
+    accessor.invoke(() -> createCache(disableSetOpToStartTx));
+    dataStore1.invoke(() -> createCache(disableSetOpToStartTx));
+    dataStore2.invoke(() -> createCache(disableSetOpToStartTx));
+    dataStore3.invoke(() -> createCache(disableSetOpToStartTx));
+
+    accessor.invoke(() -> createPR(true));
+    dataStore1.invoke(() -> createPR(false));
+    dataStore2.invoke(() -> createPR(false));
+    dataStore3.invoke(() -> createPR(false));
+  }
+
+  private void loadRegion() {
+    Region<Long, String> region = basicGetCache().getRegion(Region.SEPARATOR + REGION_NAME);
+    testData.forEach((k, v) -> region.put(k, v));
+  }
+
+  private void verifyRegionKeysetWithTx(boolean disableSetOpToStartTx) {
+    accessor.invoke(() -> verifyRegionKeysetWithTx(disableSetOpToStartTx, true));
+    dataStore1.invoke(() -> verifyRegionKeysetWithTx(disableSetOpToStartTx, false));
+    dataStore2.invoke(() -> verifyRegionKeysetWithTx(disableSetOpToStartTx, false));
+    dataStore3.invoke(() -> verifyRegionKeysetWithTx(disableSetOpToStartTx, false));
+  }
+
+  private void verifyRegionValuesWithTx(boolean disableSetOpToStartTx) {
+    accessor.invoke(() -> verifyRegionValuesWithTx(disableSetOpToStartTx, true));
+    dataStore1.invoke(() -> verifyRegionValuesWithTx(disableSetOpToStartTx, false));
+    dataStore2.invoke(() -> verifyRegionValuesWithTx(disableSetOpToStartTx, false));
+    dataStore3.invoke(() -> verifyRegionValuesWithTx(disableSetOpToStartTx, false));
+  }
+
+  private void verifyRegionEntriesWithTx(boolean disableSetOpToStartTx) {
+    accessor.invoke(() -> verifyRegionEntriesWithTx(disableSetOpToStartTx, true));
+    dataStore1.invoke(() -> verifyRegionEntriesWithTx(disableSetOpToStartTx, false));
+    dataStore2.invoke(() -> verifyRegionEntriesWithTx(disableSetOpToStartTx, false));
+    dataStore3.invoke(() -> verifyRegionEntriesWithTx(disableSetOpToStartTx, false));
+  }
+
+  private void verifyRegionKeysetWithTx(boolean disableSetOpToStartTx, boolean isAccessor) {
+    CacheTransactionManager txMgr = basicGetCache().getCacheTransactionManager();
+    Region<Long, String> region = basicGetCache().getRegion(Region.SEPARATOR + REGION_NAME);
+    try {
+      txMgr.begin();
+      Collection<Long> set = region.keySet();
+      set.forEach((key) -> assertTrue(testData.keySet().contains(key)));
+    } finally {
+      validateTXManager(disableSetOpToStartTx, isAccessor);
+      txMgr.rollback();
+    }
+  }
+
+  private void verifyRegionValuesWithTx(boolean disableSetOpToStartTx, boolean isAccessor) {
+    CacheTransactionManager txMgr = basicGetCache().getCacheTransactionManager();
+    Region<Long, String> region = basicGetCache().getRegion(Region.SEPARATOR + REGION_NAME);
+    try {
+      txMgr.begin();
+      Collection<String> set = region.values();
+      set.forEach((value) -> assertTrue(testData.values().contains(value)));
+    } finally {
+      validateTXManager(disableSetOpToStartTx, isAccessor);
+      txMgr.rollback();
+    }
+  }
+
+  private void verifyRegionEntriesWithTx(boolean disableSetOpToStartTx, boolean isAccessor) {
+    CacheTransactionManager txMgr = basicGetCache().getCacheTransactionManager();
+    Region<Long, String> region = basicGetCache().getRegion(Region.SEPARATOR + REGION_NAME);
+    try {
+      txMgr.begin();
+      Collection<Map.Entry<Long, String>> set = region.entrySet();
+      set.forEach((entry) -> {
+        assertTrue(testData.values().contains(entry.getValue()));
+        assertTrue(testData.keySet().contains(entry.getKey()));
+      });
+    } finally {
+      validateTXManager(disableSetOpToStartTx, isAccessor);
+      txMgr.rollback();
+    }
+  }
+
+  private void validateTXManager(boolean disableSetOpToStartTx, boolean isAccessor) {
+    assertNotNull(TXManagerImpl.getCurrentTXState());
+    if (disableSetOpToStartTx || isAccessor) {
+      assertFalse(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
+    } else {
+      assertTrue(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
+    }
+  }
+
+  private void verifyNoTxState() {
+    TXManagerImpl mgr = getCache().getTxManager();
+    assertEquals(0, mgr.hostedTransactionsInProgressForTest());
+  }
+
+  final String restoreSetOperationTransactionBehavior = "restoreSetOperationTransactionBehavior";
+  final String RESTORE_SET_OPERATION_PROPERTY =
+      (System.currentTimeMillis() % 2 == 0 ? DistributionConfig.GEMFIRE_PREFIX : "geode.")
+          + restoreSetOperationTransactionBehavior;
+
+  private void createCache(boolean disableSetOpToStartTx) {
+    if (disableSetOpToStartTx) {
+      logger.info("setting system property {} to true ", RESTORE_SET_OPERATION_PROPERTY);
+      System.setProperty(RESTORE_SET_OPERATION_PROPERTY, "true");
+    }
+    getCache();
+  }
+
+  private void createPR(boolean isAccessor) {
+    basicGetCache().createRegionFactory(RegionShortcut.PARTITION)
+        .setPartitionAttributes(new PartitionAttributesFactory<Long, String>().setTotalNumBuckets(3)
+            .setLocalMaxMemory(isAccessor ? 0 : 1).create())
+        .create(REGION_NAME);
+  }
+
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransaction2DUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransaction2DUnitTest.java
index 4342228..cff4057 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransaction2DUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransaction2DUnitTest.java
@@ -15,73 +15,32 @@
 package org.apache.geode.internal.cache.execute;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-
 import static org.assertj.core.api.Assertions.*;
 
-import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.CacheTransactionManager;
 import org.apache.geode.cache.PartitionAttributes;
 import org.apache.geode.cache.PartitionAttributesFactory;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionFactory;
 import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.cache.TransactionException;
-import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.cache.TXManagerImpl;
-import org.apache.geode.internal.cache.TXStateProxyImpl;
-import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.test.dunit.Host;
 import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
-import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
 import org.apache.geode.test.junit.categories.DistributedTest;
-import org.apache.logging.log4j.Logger;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
-
-import junitparams.JUnitParamsRunner;
-import junitparams.Parameters;
 
 @Category(DistributedTest.class)
-@RunWith(JUnitParamsRunner.class)
-@SuppressWarnings("serial")
 public class PRTransaction2DUnitTest extends JUnit4CacheTestCase {
-
-  private static final Logger logger = LogService.getLogger();
-
-  @Rule
-  public DistributedRestoreSystemProperties restoreSystemProperties =
-      new DistributedRestoreSystemProperties();
-
-  VM accessor = null;
-  VM dataStore1 = null;
-  VM dataStore2 = null;
-  VM dataStore3 = null;
-  String regionName = "region";
-  String region2Name = "region2";
-
-  @Override
-  public final void postSetUp() throws Exception {
-    disconnectAllFromDS();
-    Host host = Host.getHost(0);
-    dataStore1 = host.getVM(0);
-    dataStore2 = host.getVM(1);
-    dataStore3 = host.getVM(2);
-    accessor = host.getVM(3);
-  }
+  Host host = Host.getHost(0);
+  VM dataStore1 = host.getVM(0);
+  VM dataStore2 = host.getVM(1);
 
   @Test
   public void testSizeOpOnLocalRegionInTransaction() {
+    String regionName = "region";
+    String region2Name = "region2";
     int totalBuckets = 2;
     boolean isSecondRegionLocal = true;
     setupRegions(totalBuckets, regionName, isSecondRegionLocal, region2Name);
@@ -92,6 +51,8 @@ public class PRTransaction2DUnitTest extends JUnit4CacheTestCase {
 
   @Test
   public void testSizeOpOnReplicateRegionInTransaction() {
+    String regionName = "region";
+    String region2Name = "region2";
     int totalBuckets = 2;
     boolean isSecondRegionLocal = false;
     setupRegions(totalBuckets, regionName, isSecondRegionLocal, region2Name);
@@ -118,16 +79,11 @@ public class PRTransaction2DUnitTest extends JUnit4CacheTestCase {
     });
   }
 
-  private void createPartitionedRegion(String regionName, int copies, int totalBuckets) {
-    createPartitionedRegion(regionName, copies, totalBuckets, false);
-  }
-
   @SuppressWarnings("rawtypes")
-  private void createPartitionedRegion(String regionName, int copies, int totalBuckets,
-      boolean isAccessor) {
+  private void createPartitionedRegion(String regionName, int copies, int totalBuckets) {
     RegionFactory<Integer, String> factory = getCache().createRegionFactory();
     PartitionAttributes pa = new PartitionAttributesFactory().setTotalNumBuckets(totalBuckets)
-        .setRedundantCopies(copies).setLocalMaxMemory(isAccessor ? 0 : 1).create();
+        .setRedundantCopies(copies).create();
     factory.setPartitionAttributes(pa).create(regionName);
   }
 
@@ -186,146 +142,4 @@ public class PRTransaction2DUnitTest extends JUnit4CacheTestCase {
     }
   }
 
-  long k1 = 1;
-  long k2 = 2;
-  long k3 = 3;
-  long k4 = 4;
-  String v1 = "value1";
-  String v2 = "value2";
-  String v3 = "value3";
-
-  enum SetOp {
-    KEYSET, VALUES, ENTRYSET;
-  }
-
-  protected void createRegion(boolean preventSetOpToStartTx) {
-    accessor.invoke(() -> createCache(preventSetOpToStartTx));
-    dataStore1.invoke(() -> createCache(preventSetOpToStartTx));
-    dataStore2.invoke(() -> createCache(preventSetOpToStartTx));
-    dataStore3.invoke(() -> createCache(preventSetOpToStartTx));
-
-    int totalBuckets = 3;
-    accessor.invoke(() -> createPartitionedRegion(regionName, 0, totalBuckets, true));
-    dataStore1.invoke(() -> createPartitionedRegion(regionName, 0, totalBuckets));
-    dataStore2.invoke(() -> createPartitionedRegion(regionName, 0, totalBuckets));
-    dataStore3.invoke(() -> createPartitionedRegion(regionName, 0, totalBuckets));
-  }
-
-  @Test
-  @Parameters
-  public void testRegionSetOpWithTx(SetOp op, boolean preventSetOpToStartTx) {
-    verifySetOp(op, preventSetOpToStartTx);
-  }
-
-  private Object[] parametersForTestRegionSetOpWithTx() {
-    return new Object[] {new Object[] {SetOp.VALUES, false}, new Object[] {SetOp.VALUES, true},
-        new Object[] {SetOp.KEYSET, false}, new Object[] {SetOp.KEYSET, true},
-        new Object[] {SetOp.ENTRYSET, false}, new Object[] {SetOp.ENTRYSET, true},};
-  }
-
-  @SuppressWarnings("rawtypes")
-  private void basicVerifySetOp(SetOp op, boolean preventSetOpToStartTx, boolean isAccessor) {
-    Cache cache = basicGetCache();
-    Region<Long, String> region = cache.getRegion(Region.SEPARATOR + regionName);
-
-    Collection<Long> keys = new ArrayList<Long>();
-    keys.add(k1);
-    keys.add(k2);
-    keys.add(k3);
-    keys.add(k4);
-    Collection<String> values = new ArrayList<String>();
-    values.add(v1);
-    values.add(v2);
-    values.add(v3);
-    CacheTransactionManager txMgr = basicGetCache().getCacheTransactionManager();
-
-    try {
-      txMgr.begin();
-
-      Collection set = getSetOp(region, op);
-
-      verifySetOp(op, region, keys, values, set);
-    } finally {
-      assertNotNull(TXManagerImpl.getCurrentTXState());
-      if (preventSetOpToStartTx || isAccessor) {
-        assertFalse(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
-      } else {
-        assertTrue(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
-      }
-      txMgr.rollback();
-    }
-  }
-
-  private void verifySetOp(SetOp op, boolean preventSetOpToStartTx) {
-    createRegion(preventSetOpToStartTx);
-    dataStore1.invoke(() -> initRegion());
-
-    accessor.invoke(() -> basicVerifySetOp(op, preventSetOpToStartTx, true));
-    dataStore1.invoke(() -> basicVerifySetOp(op, preventSetOpToStartTx, false));
-    dataStore2.invoke(() -> basicVerifySetOp(op, preventSetOpToStartTx, false));
-    dataStore3.invoke(() -> basicVerifySetOp(op, preventSetOpToStartTx, false));
-  }
-
-  @SuppressWarnings("rawtypes")
-  private void verifySetOp(SetOp op, Region<Long, String> region, Collection<Long> keys,
-      Collection<String> values, Collection set) {
-    Iterator it = set.iterator();
-    while (it.hasNext()) {
-      Object o = it.next();
-      switch (op) {
-        case VALUES:
-          assertTrue(values.contains(o));
-          break;
-        case KEYSET:
-          assertTrue(keys.contains(o));
-          break;
-        case ENTRYSET:
-          assertTrue(keys.contains(((Region.Entry) o).getKey()));
-          assertTrue(values.contains(((Region.Entry) o).getValue()));
-          break;
-        default:
-          fail("Unexpected op: " + op);
-      }
-    }
-  }
-
-  @SuppressWarnings("rawtypes")
-  private Collection getSetOp(Region<Long, String> region, SetOp op) {
-    Collection set = null;
-    switch (op) {
-      case VALUES:
-        set = region.values();
-        break;
-      case KEYSET:
-        set = region.keySet();
-        break;
-      case ENTRYSET:
-        set = region.entrySet();
-        break;
-      default:
-        fail("Unexpected op: " + op);
-    }
-    return set;
-  }
-
-  private void initRegion() {
-    Region<Long, String> currRegion;
-    currRegion = basicGetCache().getRegion(Region.SEPARATOR + regionName);
-    currRegion.put(k1, v1);
-    currRegion.put(k2, v2);
-    currRegion.put(k3, v3);
-  }
-
-  final String preventSetOpBootstrapTransaction = "preventSetOpBootstrapTransaction";
-  final String PREVENT_SET_OP_BOOTSTRAP_TRANSACTION =
-      (System.currentTimeMillis() % 2 == 0 ? DistributionConfig.GEMFIRE_PREFIX : "geode.")
-          + preventSetOpBootstrapTransaction;
-
-  private void createCache(boolean preventSetOpToStartTx) {
-    if (preventSetOpToStartTx) {
-      logger.info("setting system property {} to true ", PREVENT_SET_OP_BOOTSTRAP_TRANSACTION);
-      System.setProperty(PREVENT_SET_OP_BOOTSTRAP_TRANSACTION, "true");
-    }
-    getCache();
-  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/internal/jta/JtaIntegrationJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/jta/JtaIntegrationJUnitTest.java
index b6a7fab..07cdc6c 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/jta/JtaIntegrationJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/jta/JtaIntegrationJUnitTest.java
@@ -15,50 +15,29 @@
 package org.apache.geode.internal.jta;
 
 import org.apache.geode.cache.*;
-import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import org.apache.geode.internal.cache.TXManagerImpl;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.logging.log4j.Logger;
 import org.junit.After;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
 
-import junitparams.JUnitParamsRunner;
-import junitparams.Parameters;
-
-import javax.naming.Context;
-import javax.naming.NamingException;
-import javax.transaction.NotSupportedException;
-import javax.transaction.SystemException;
 import javax.transaction.Transaction;
 import javax.transaction.TransactionManager;
-import javax.transaction.UserTransaction;
 
 import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
 import static org.junit.Assert.*;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-
 /**
  * Moved some non-DUnit tests over from org/apache/geode/internal/jta/dunit/JTADUnitTest
  * 
  */
 @Category(IntegrationTest.class)
-@RunWith(JUnitParamsRunner.class)
 public class JtaIntegrationJUnitTest {
 
   private static final Logger logger = LogService.getLogger();
 
-  @Rule
-  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
-
   @After
   public void tearDown() {
     InternalDistributedSystem ids = InternalDistributedSystem.getAnyInstance();
@@ -145,148 +124,4 @@ public class JtaIntegrationJUnitTest {
       CacheUtils.closeCache();
     }
   }
-
-  @Test
-  @Parameters
-  public void testRegionSetOpWithJTA(SetOp op, boolean preventSetOpToStartJTA)
-      throws NotSupportedException, SystemException, NamingException {
-    verifyJTASetOp(op, preventSetOpToStartJTA);
-  }
-
-  private Object[] parametersForTestRegionSetOpWithJTA() {
-    return new Object[] {new Object[] {SetOp.VALUES, false}, new Object[] {SetOp.VALUES, true},
-        new Object[] {SetOp.KEYSET, false}, new Object[] {SetOp.KEYSET, true},
-        new Object[] {SetOp.ENTRYSET, false}, new Object[] {SetOp.ENTRYSET, true},};
-  }
-
-  long k1 = 1;
-  long k2 = 2;
-  long k3 = 3;
-  long k4 = 4;
-  String v1 = "value1";
-  String v2 = "value2";
-  String v3 = "value3";
-  String regionName = "region1";
-
-  enum SetOp {
-    KEYSET, VALUES, ENTRYSET;
-  }
-
-  private void verifyJTASetOp(SetOp op, boolean preventSetOpToStartJTA)
-      throws NotSupportedException, SystemException, NamingException {
-    Cache cache = createCache(preventSetOpToStartJTA);
-    Region<Long, String> region = createRegion(cache);
-    initRegion(region);
-
-    basicVerifySetOp(op, preventSetOpToStartJTA, cache);
-  }
-
-  protected Region<Long, String> createRegion(Cache cache) {
-    RegionFactory<Long, String> rf = cache.createRegionFactory(RegionShortcut.REPLICATE);
-    Region<Long, String> r = rf.create(regionName);
-    return r;
-  }
-
-  final String preventSetOpBootstrapTransaction = "preventSetOpBootstrapTransaction";
-  final String PREVENT_SET_OP_BOOTSTRAP_TRANSACTION =
-      (System.currentTimeMillis() % 2 == 0 ? DistributionConfig.GEMFIRE_PREFIX : "geode.")
-          + preventSetOpBootstrapTransaction;
-
-  private Cache createCache(boolean preventSetOpToStartJTA) {
-    if (preventSetOpToStartJTA) {
-      logger.info("setting system property {} to true ", PREVENT_SET_OP_BOOTSTRAP_TRANSACTION);
-      System.setProperty(PREVENT_SET_OP_BOOTSTRAP_TRANSACTION, "true");
-    }
-    CacheFactory cf = new CacheFactory().set(MCAST_PORT, "0");
-    Cache cache = cf.create();
-    return cache;
-  }
-
-  private void initRegion(Region<Long, String> region) {
-    region.put(k1, v1);
-    region.put(k2, v2);
-    region.put(k3, v3);
-    region.put(k4, v3);
-  }
-
-  private void basicVerifySetOp(SetOp op, boolean preventSetOpToStartJTA, Cache cache)
-      throws NotSupportedException, SystemException, NamingException {
-    Region<Long, String> region = cache.getRegion(Region.SEPARATOR + regionName);
-
-    Context ctx = cache.getJNDIContext();
-    UserTransaction ta = startUserTransaction(ctx);
-
-    Collection<Long> keys = new ArrayList<Long>();
-    keys.add(k1);
-    keys.add(k2);
-    keys.add(k3);
-    keys.add(k4);
-    Collection<String> values = new ArrayList<String>();
-    values.add(v1);
-    values.add(v2);
-    values.add(v3);
-
-    // Begin the user transaction
-    try {
-      ta.begin();
-
-      @SuppressWarnings("rawtypes")
-      Collection set = getSetOp(region, op);
-
-      verifyJTASetOp(op, region, keys, values, set);
-    } finally {
-      if (preventSetOpToStartJTA) {
-        assertNull(TXManagerImpl.getCurrentTXState());
-      } else {
-        assertNotNull(TXManagerImpl.getCurrentTXState());
-        ta.rollback();
-      }
-    }
-  }
-
-  private UserTransaction startUserTransaction(Context ctx) throws NamingException {
-    return (UserTransaction) ctx.lookup("java:/UserTransaction");
-  }
-
-  @SuppressWarnings("rawtypes")
-  private Collection getSetOp(Region<Long, String> region, SetOp op) {
-    Collection set = null;
-    switch (op) {
-      case VALUES:
-        set = region.values();
-        break;
-      case KEYSET:
-        set = region.keySet();
-        break;
-      case ENTRYSET:
-        set = region.entrySet();
-        break;
-      default:
-        fail("Unexpected op: " + op);
-    }
-    return set;
-  }
-
-  @SuppressWarnings("rawtypes")
-  private void verifyJTASetOp(SetOp op, Region<Long, String> region, Collection<Long> keys,
-      Collection<String> values, Collection set) {
-    Iterator it = set.iterator();
-    while (it.hasNext()) {
-      Object o = it.next();
-      switch (op) {
-        case VALUES:
-          assertTrue(values.contains(o));
-          break;
-        case KEYSET:
-          assertTrue(keys.contains(o));
-          break;
-        case ENTRYSET:
-          assertTrue(keys.contains(((Region.Entry) o).getKey()));
-          assertTrue(values.contains(((Region.Entry) o).getValue()));
-          break;
-        default:
-          fail("Unexpected op: " + op);
-      }
-    }
-  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/internal/jta/SetOperationJTAJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/jta/SetOperationJTAJUnitTest.java
new file mode 100644
index 0000000..2666ed5
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/jta/SetOperationJTAJUnitTest.java
@@ -0,0 +1,183 @@
+/*
+ * 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.jta;
+
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.naming.Context;
+import javax.transaction.UserTransaction;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.cache.TXManagerImpl;
+import org.apache.geode.internal.cache.TXStateProxyImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.logging.log4j.Logger;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+
+@Category(IntegrationTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class SetOperationJTAJUnitTest {
+  private static final Logger logger = LogService.getLogger();
+  private static final String REGION_NAME = "region1";
+
+  private Map<Long, String> testData;
+  private Cache cache;
+
+  @Rule
+  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
+
+  @Before
+  public void setup() {
+    testData = new HashMap<>();
+    testData.put(1L, "value1");
+    testData.put(2L, "value2");
+    testData.put(3L, "duplicateValue");
+    testData.put(4L, "duplicateValue");
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    closeCache();
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionKeysetWithJTA(boolean disableSetOpToStartJTA) throws Exception {
+    Region<Long, String> region = setupAndLoadRegion(disableSetOpToStartJTA);
+    Context ctx = cache.getJNDIContext();
+    UserTransaction userTX = startUserTransaction(ctx);
+    try {
+      userTX.begin();
+      Collection<Long> set = region.keySet();
+      set.forEach((key) -> assertTrue(testData.keySet().contains(key)));
+    } finally {
+      validateTXManager(disableSetOpToStartJTA);
+      if (!disableSetOpToStartJTA) {
+        userTX.rollback();
+      }
+    }
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionValuesWithJTA(boolean disableSetOpToStartJTA) throws Exception {
+    Region<Long, String> region = setupAndLoadRegion(disableSetOpToStartJTA);
+    Context ctx = cache.getJNDIContext();
+    UserTransaction userTX = startUserTransaction(ctx);
+    try {
+      userTX.begin();
+      Collection<String> set = region.values();
+      set.forEach((value) -> assertTrue(testData.values().contains(value)));
+    } finally {
+      validateTXManager(disableSetOpToStartJTA);
+      if (!disableSetOpToStartJTA) {
+        userTX.rollback();
+      }
+    }
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void testRegionEntriesWithJTA(boolean disableSetOpToStartJTA) throws Exception {
+    Region<Long, String> region = setupAndLoadRegion(disableSetOpToStartJTA);
+    Context ctx = cache.getJNDIContext();
+    UserTransaction userTX = startUserTransaction(ctx);
+    try {
+      userTX.begin();
+      Collection<Map.Entry<Long, String>> set = region.entrySet();
+      set.forEach((entry) -> {
+        assertTrue(testData.values().contains(entry.getValue()));
+        assertTrue(testData.keySet().contains(entry.getKey()));
+      });
+    } finally {
+      validateTXManager(disableSetOpToStartJTA);
+      if (!disableSetOpToStartJTA) {
+        userTX.rollback();
+      }
+    }
+  }
+
+  private Region<Long, String> setupAndLoadRegion(boolean disableSetOpToStartTx) {
+    this.cache = createCache(disableSetOpToStartTx);
+    Region<Long, String> region = createRegion(cache);
+    testData.forEach((k, v) -> region.put(k, v));
+    return region;
+  }
+
+  private UserTransaction startUserTransaction(Context ctx) throws Exception {
+    return (UserTransaction) ctx.lookup("java:/UserTransaction");
+  }
+
+  private void validateTXManager(boolean disableSetOpToStartTx) {
+    if (disableSetOpToStartTx) {
+      assertNull(TXManagerImpl.getCurrentTXState());
+    } else {
+      assertNotNull(TXManagerImpl.getCurrentTXState());
+      assertTrue(((TXStateProxyImpl) TXManagerImpl.getCurrentTXState()).hasRealDeal());
+    }
+  }
+
+  protected Region<Long, String> createRegion(Cache cache) {
+    RegionFactory<Long, String> rf = cache.createRegionFactory(RegionShortcut.REPLICATE);
+    Region<Long, String> r = rf.create(REGION_NAME);
+    return r;
+  }
+
+  final String restoreSetOperationTransactionBehavior = "restoreSetOperationTransactionBehavior";
+  final String RESTORE_SET_OPERATION_PROPERTY =
+      (System.currentTimeMillis() % 2 == 0 ? DistributionConfig.GEMFIRE_PREFIX : "geode.")
+          + restoreSetOperationTransactionBehavior;
+
+  private Cache createCache(boolean disableSetOpToStartJTA) {
+    if (disableSetOpToStartJTA) {
+      logger.info("setting system property {} to true ", RESTORE_SET_OPERATION_PROPERTY);
+      System.setProperty(RESTORE_SET_OPERATION_PROPERTY, "true");
+    }
+    CacheFactory cf = new CacheFactory().set(MCAST_PORT, "0");
+    this.cache = (GemFireCacheImpl) cf.create();
+    return this.cache;
+  }
+
+  protected void closeCache() {
+    if (this.cache != null) {
+      Cache c = this.cache;
+      this.cache = null;
+      c.close();
+    }
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/lang/SystemPropertyHelperJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/lang/SystemPropertyHelperJUnitTest.java
index f5afc43..53f5655 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/lang/SystemPropertyHelperJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/lang/SystemPropertyHelperJUnitTest.java
@@ -22,33 +22,33 @@ import org.junit.experimental.categories.Category;
 
 @Category(UnitTest.class)
 public class SystemPropertyHelperJUnitTest {
-  String preventSetOpBootstrapTransaction = "preventSetOpBootstrapTransaction";
+  String restoreSetOperationTransactionBehavior = "restoreSetOperationTransactionBehavior";
 
   @Test
-  public void testPreventSetOpBootstrapTransactionDefaultToFalse() {
-    assertFalse(SystemPropertyHelper.preventSetOpBootstrapTransaction());
+  public void testRestoreSetOperationTransactionBehaviorDefaultToFalse() {
+    assertFalse(SystemPropertyHelper.restoreSetOperationTransactionBehavior());
   }
 
   @Test
-  public void testPreventSetOpBootstrapTransactionSystemProperty() {
-    String gemfirePrefixProperty = "gemfire." + preventSetOpBootstrapTransaction;
+  public void testRestoreSetOperationTransactionBehaviorSystemProperty() {
+    String gemfirePrefixProperty = "gemfire." + restoreSetOperationTransactionBehavior;
     System.setProperty(gemfirePrefixProperty, "true");
-    assertTrue(SystemPropertyHelper.preventSetOpBootstrapTransaction());
+    assertTrue(SystemPropertyHelper.restoreSetOperationTransactionBehavior());
     System.clearProperty(gemfirePrefixProperty);
 
-    String geodePrefixProperty = "geode." + preventSetOpBootstrapTransaction;
+    String geodePrefixProperty = "geode." + restoreSetOperationTransactionBehavior;
     System.setProperty(geodePrefixProperty, "true");
-    assertTrue(SystemPropertyHelper.preventSetOpBootstrapTransaction());
+    assertTrue(SystemPropertyHelper.restoreSetOperationTransactionBehavior());
     System.clearProperty(geodePrefixProperty);
   }
 
   @Test
-  public void testPreventSetOpBootstrapTransactionGeodePreference() {
-    String gemfirePrefixProperty = "gemfire." + preventSetOpBootstrapTransaction;
-    String geodePrefixProperty = "geode." + preventSetOpBootstrapTransaction;
+  public void testRestoreSetOperationTransactionBehaviorGeodePreference() {
+    String gemfirePrefixProperty = "gemfire." + restoreSetOperationTransactionBehavior;
+    String geodePrefixProperty = "geode." + restoreSetOperationTransactionBehavior;
     System.setProperty(geodePrefixProperty, "false");
     System.setProperty(gemfirePrefixProperty, "true");
-    assertFalse(SystemPropertyHelper.preventSetOpBootstrapTransaction());
+    assertFalse(SystemPropertyHelper.restoreSetOperationTransactionBehavior());
     System.clearProperty(geodePrefixProperty);
     System.clearProperty(gemfirePrefixProperty);
   }

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <commits@geode.apache.org>'].

Mime
View raw message