You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by es...@apache.org on 2017/10/24 23:10:47 UTC

[geode] branch develop updated: Feature/geode 3897 (#968)

This is an automated email from the ASF dual-hosted git repository.

eshu11 pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 648d172  Feature/geode 3897 (#968)
648d172 is described below

commit 648d1724a94f56acebde48834aeaf203a5e4f98e
Author: pivotal-eshu <es...@pivotal.io>
AuthorDate: Tue Oct 24 16:10:44 2017 -0700

    Feature/geode 3897 (#968)
    
    * GEODE-3897: Use internalSuspend before start a new transaction.
    
      * When registering pdx in a new transaction, product should internal suspend an existing transaction.
      * Add a unit test that fails without this fix.
---
 .../apache/geode/internal/cache/TXManagerImpl.java |  17 +--
 .../geode/pdx/internal/PeerTypeRegistration.java   |   5 +-
 .../apache/geode/pdx/PdxSerializableDUnitTest.java | 122 +++++++++++----------
 3 files changed, 74 insertions(+), 70 deletions(-)

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 ebd37cc..f35e4ea 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
@@ -685,7 +685,8 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
 
   /**
    * 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.
+   * transaction. The same thread is expected to unpause/resume the transaction later. The thread
+   * should not start a new transaction after it paused a transaction.
    *
    * @return the state of the transaction or null. Pass this value to
    *         {@link TXManagerImpl#unpauseTransaction} to reactivate the puased/suspended
@@ -696,14 +697,12 @@ 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. Currently only used in testing.
+   * If the current thread is in a transaction then internal suspend will cause it to no longer be
+   * in a transaction. The thread can start a new transaction after it internal suspended a
+   * 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);
   }
@@ -745,15 +744,11 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
 
   /**
    * Activates the specified transaction on the calling thread. Does not require the same thread to
-   * resume it. Currently only used in testing.
+   * resume it.
    *
    * @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);
   }
diff --git a/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java b/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
index ad56b78..13c97fa 100644
--- a/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
+++ b/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
@@ -618,13 +618,14 @@ public class PeerTypeRegistration implements TypeRegistration {
   private TXStateProxy suspendTX() {
     InternalCache cache = (InternalCache) getIdToType().getRegionService();
     TXManagerImpl txManager = (TXManagerImpl) cache.getCacheTransactionManager();
-    return txManager.pauseTransaction();
+    // A new transaction will be started to register pdx.
+    return txManager.internalSuspend();
   }
 
   private void resumeTX(TXStateProxy state) {
     if (state != null) {
       TXManagerImpl txManager = state.getTxMgr();
-      txManager.unpauseTransaction(state);
+      txManager.internalResume(state);
     }
   }
 
diff --git a/geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableDUnitTest.java b/geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableDUnitTest.java
index 497f428..1008702 100644
--- a/geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableDUnitTest.java
@@ -24,6 +24,7 @@ import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.CacheEvent;
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.PartitionAttributesFactory;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.cache.Scope;
@@ -58,6 +59,9 @@ import java.util.concurrent.TimeoutException;
 @Category({DistributedTest.class, SerializationTest.class})
 public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
 
+  private static final SimpleClass ANOBJECT = new SimpleClass(57, (byte) 3);
+  private static final String TEST_REGION_NAME = "testSimplePdx";
+
   @Test
   public void testSimplePut() {
     Host host = Host.getHost(0);
@@ -70,7 +74,7 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
         AttributesFactory af = new AttributesFactory();
         af.setScope(Scope.DISTRIBUTED_ACK);
         af.setDataPolicy(DataPolicy.REPLICATE);
-        createRootRegion("testSimplePdx", af.create());
+        createRootRegion(TEST_REGION_NAME, af.create());
         return null;
       }
     };
@@ -78,25 +82,9 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
     vm1.invoke(createRegion);
     vm2.invoke(createRegion);
     vm3.invoke(createRegion);
-    vm1.invoke(new SerializableCallable() {
-      public Object call() throws Exception {
-        // Check to make sure the type region is not yet created
-        Region r = getRootRegion("testSimplePdx");
-        r.put(1, new SimpleClass(57, (byte) 3));
-        // Ok, now the type registry should exist
-        assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
-        return null;
-      }
-    });
-    vm2.invoke(new SerializableCallable() {
-      public Object call() throws Exception {
-        // Ok, now the type registry should exist
-        assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
-        Region r = getRootRegion("testSimplePdx");
-        assertEquals(new SimpleClass(57, (byte) 3), r.get(1));
-        return null;
-      }
-    });
+
+    vm1.invoke(() -> doSimplePut(false));
+    vm2.invoke(() -> verifySimplePut());
 
     vm3.invoke(new SerializableCallable("check for PDX") {
 
@@ -107,6 +95,31 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
     });
   }
 
+  private void createPR() {
+    getCache().createRegionFactory(RegionShortcut.PARTITION)
+        .setPartitionAttributes(new PartitionAttributesFactory<Integer, Object>().create())
+        .create(TEST_REGION_NAME);
+  }
+
+  @Test
+  public void testSimplePutOnPRWithTx() {
+    Host host = Host.getHost(0);
+    VM vm1 = host.getVM(0);
+    VM vm2 = host.getVM(1);
+    VM vm3 = host.getVM(2);
+
+    vm1.invoke(() -> createPR());
+    vm2.invoke(() -> createPR());
+    vm3.invoke(() -> createPR());
+    vm1.invoke(() -> doSimplePut(true));
+
+    vm2.invoke(() -> verifySimplePut());
+
+    vm3.invoke(() -> {
+      assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
+    });
+  }
+
   @Test
   public void testTransactionCallbacksNotInvoked() {
     Host host = Host.getHost(0);
@@ -118,7 +131,7 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
         AttributesFactory af = new AttributesFactory();
         af.setScope(Scope.DISTRIBUTED_ACK);
         af.setDataPolicy(DataPolicy.REPLICATE);
-        createRootRegion("testSimplePdx", af.create());
+        createRootRegion(TEST_REGION_NAME, af.create());
         addPoisonedTransactionListeners();
         return null;
       }
@@ -126,28 +139,8 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
 
     vm1.invoke(createRegionAndAddPoisonedListener);
     vm2.invoke(createRegionAndAddPoisonedListener);
-    vm1.invoke(new SerializableCallable() {
-      public Object call() throws Exception {
-        // Check to make sure the type region is not yet created
-        Region r = getRootRegion("testSimplePdx");
-        Cache mycache = getCache();
-        mycache.getCacheTransactionManager().begin();
-        r.put(1, new SimpleClass(57, (byte) 3));
-        mycache.getCacheTransactionManager().commit();
-        // Ok, now the type registry should exist
-        assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
-        return null;
-      }
-    });
-    vm2.invoke(new SerializableCallable() {
-      public Object call() throws Exception {
-        // Ok, now the type registry should exist
-        assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
-        Region r = getRootRegion("testSimplePdx");
-        assertEquals(new SimpleClass(57, (byte) 3), r.get(1));
-        return null;
-      }
-    });
+    vm1.invoke(() -> doSimplePut(true));
+    vm2.invoke(() -> verifySimplePut());
   }
 
   @Test
@@ -162,7 +155,7 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
         AttributesFactory af = new AttributesFactory();
         af.setScope(Scope.DISTRIBUTED_ACK);
         af.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
-        createRootRegion("testSimplePdx", af.create());
+        createRootRegion(TEST_REGION_NAME, af.create());
         return null;
       }
     };
@@ -185,7 +178,7 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
         af.setScope(Scope.DISTRIBUTED_ACK);
         af.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
         af.setDiskStoreName("store1");
-        createRootRegion("testSimplePdx", af.create());
+        createRootRegion(TEST_REGION_NAME, af.create());
         return null;
       }
     };
@@ -201,21 +194,12 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
     vm1.invoke(createRegion);
     vm2.invoke(createRegion);
 
-    vm1.invoke(new SerializableCallable() {
-      public Object call() throws Exception {
-        // Check to make sure the type region is not yet created
-        Region r = getRootRegion("testSimplePdx");
-        r.put(1, new SimpleClass(57, (byte) 3));
-        // Ok, now the type registry should exist
-        assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
-        return null;
-      }
-    });
+    vm1.invoke(() -> doSimplePut(false));
 
     final SerializableCallable checkForObject = new SerializableCallable() {
       public Object call() throws Exception {
-        Region r = getRootRegion("testSimplePdx");
-        assertEquals(new SimpleClass(57, (byte) 3), r.get(1));
+        Region r = getRootRegion(TEST_REGION_NAME);
+        assertEquals(ANOBJECT, r.get(1));
         // Ok, now the type registry should exist
         assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
         return null;
@@ -440,6 +424,30 @@ public class PdxSerializableDUnitTest extends JUnit4CacheTestCase {
   }
 
 
+  private void doSimplePut(boolean useTransaction) {
+    // Check to make sure the type region is not yet created
+    Region r = getRootRegion(TEST_REGION_NAME);
+    if (useTransaction) {
+      getCache().getCacheTransactionManager().begin();
+    }
+    try {
+      r.put(1, ANOBJECT);
+    } finally {
+      if (useTransaction) {
+        getCache().getCacheTransactionManager().commit();
+      }
+    }
+    // Ok, now the type registry should exist
+    assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
+  }
+
+  private void verifySimplePut() {
+    // Ok, now the type registry should exist
+    assertNotNull(getRootRegion(PeerTypeRegistration.REGION_NAME));
+    Region r = getRootRegion(TEST_REGION_NAME);
+    assertEquals(ANOBJECT, r.get(1));
+  }
+
   static private class MyTestTransactionListener implements TransactionWriter, TransactionListener {
     private MyTestTransactionListener() {
 

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