Changes to correct backward compatibility for reservoir regions

- skip send/persistence of create/drop reservoir region procedure if the operation is unsuccessful locally - avoid heap dump on OOM with junit tests that causes many hprofs due to deliberate CRITICAL_UP tests
TIBCOSoftware · Jan 15, 2019 · 7ee801f · 7ee801f
1 parent b745f85
commit 7ee801f
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 32 deletions.
diff --git a/build.gradle b/build.gradle
@@ -587,8 +587,7 @@ gradle.taskGraph.whenReady { graph ->
       def test = (Test)task
       test.configure {
 
-        jvmArgs = ['-XX:+HeapDumpOnOutOfMemoryError',
-                   '-XX:+UseParNewGC', '-XX:+UseConcMarkSweepGC',
+        jvmArgs = ['-XX:+UseParNewGC', '-XX:+UseConcMarkSweepGC',
                    '-XX:CMSInitiatingOccupancyFraction=50',
                    '-XX:+CMSClassUnloadingEnabled', '-ea']
 

diff --git a/gemfirexd/core/src/main/java/com/pivotal/gemfirexd/internal/engine/Misc.java b/gemfirexd/core/src/main/java/com/pivotal/gemfirexd/internal/engine/Misc.java
@@ -395,21 +395,15 @@ public static PartitionedRegion createReservoirRegionForSampleTable(
 
   public static PartitionedRegion getReservoirRegionForSampleTable(String reservoirRegionName) {
     if (reservoirRegionName != null) {
-      GemFireCacheImpl cache = GemFireCacheImpl.getInstance();
+      GemFireCacheImpl cache = GemFireCacheImpl.getExisting();
       Region<?, ?> childRegion = cache.getRegion(reservoirRegionName);
       if (childRegion != null) {
-        return (PartitionedRegion) childRegion;
+        return (PartitionedRegion)childRegion;
       }
     }
     return null;
   }
 
-  public static void dropReservoirRegionForSampleTable(PartitionedRegion reservoirRegion) {
-    if (reservoirRegion != null) {
-      reservoirRegion.destroyRegion(null);
-    }
-  }
-
   public static PartitionedRegion.PRLocalScanIterator
   getLocalBucketsIteratorForSampleTable(PartitionedRegion reservoirRegion,
       Set<Integer> bucketSet, boolean fetchFromRemote) {

diff --git a/...src/main/java/com/pivotal/gemfirexd/internal/engine/ddl/catalog/GfxdSystemProcedures.java b/...src/main/java/com/pivotal/gemfirexd/internal/engine/ddl/catalog/GfxdSystemProcedures.java
@@ -1568,22 +1568,45 @@ public static void CREATE_OR_DROP_RESERVOIR_REGION(String reservoirRegionName,
           schema, currentUser);
 
       // first create/drop locally
-      final Object[] args = new Object[] { reservoirRegionName,
-          resolvedBaseName, isDrop };
-      GfxdSystemProcedureMessage.SysProcMethod.createOrDropReservoirRegion
-          .processMessage(args, Misc.getMyId());
-
-      // send to other nodes
-      publishMessage(args, false,
-          GfxdSystemProcedureMessage.SysProcMethod.createOrDropReservoirRegion,
-          true, false);
+      if (createOrDropReservoirRegion(reservoirRegionName, resolvedBaseName, isDrop)) {
+        // don't send to other nodes or persist if local operation is unsuccessful
+        final Object[] args = new Object[] { reservoirRegionName,
+            resolvedBaseName, isDrop };
+        // send to other nodes
+        publishMessage(args, false,
+            GfxdSystemProcedureMessage.SysProcMethod.createOrDropReservoirRegion,
+            true, false);
+      }
     } catch (StandardException se) {
       throw PublicAPI.wrapStandardException(se);
     } catch (Throwable t) {
       throw TransactionResourceImpl.wrapInSQLException(t);
     }
   }
 
+  public static boolean createOrDropReservoirRegion(String reservoirRegionName,
+      String resolvedBaseName, boolean isDrop) {
+    PartitionedRegion existingRegion = Misc.getReservoirRegionForSampleTable(
+        reservoirRegionName);
+    if (isDrop) {
+      if (existingRegion != null) {
+        existingRegion.destroyRegion(null);
+        return true;
+      } else {
+        return false;
+      }
+    } else if (existingRegion == null) {
+      PartitionedRegion region = Misc.createReservoirRegionForSampleTable(
+          reservoirRegionName, resolvedBaseName);
+      if (Misc.initialDDLReplayDone()) {
+        Assert.assertTrue(region != null);
+      }
+      return true;
+    } else {
+      return false;
+    }
+  }
+
   /**
    * Create all buckets in the given table.
    *

diff --git a/...om/pivotal/gemfirexd/internal/engine/ddl/catalog/messages/GfxdSystemProcedureMessage.java b/...om/pivotal/gemfirexd/internal/engine/ddl/catalog/messages/GfxdSystemProcedureMessage.java
@@ -33,7 +33,6 @@
 import com.gemstone.gemfire.distributed.internal.DistributionManager;
 import com.gemstone.gemfire.distributed.internal.DistributionStats;
 import com.gemstone.gemfire.distributed.internal.ReplyException;
-import com.gemstone.gemfire.internal.Assert;
 import com.gemstone.gemfire.internal.GFToSlf4jBridge;
 import com.gemstone.gemfire.internal.InternalDataSerializer;
 import com.gemstone.gemfire.internal.NanoTimer;
@@ -42,7 +41,6 @@
 import com.gemstone.gemfire.internal.cache.DiskStoreImpl;
 import com.gemstone.gemfire.internal.cache.GemFireCacheImpl;
 import com.gemstone.gemfire.internal.cache.LocalRegion;
-import com.gemstone.gemfire.internal.cache.PartitionedRegion;
 import com.gemstone.gemfire.internal.shared.ClientSharedUtils;
 import com.gemstone.gemfire.internal.snappy.CallbackFactoryProvider;
 import com.gemstone.gemfire.internal.util.ArrayUtils;
@@ -1548,17 +1546,8 @@ public void processMessage(Object[] params, DistributedMember sender) {
             "GfxdSystemProcedureMessage:CREATE_OR_DROP_RESERVOIR_REGION " +
                 "reservoirRegionName=" + reservoirRegionName +
                 " resolvedBaseName=" + resolvedBaseName + " isDrop=" + isDrop);
-        if (isDrop) {
-          PartitionedRegion region = Misc.getReservoirRegionForSampleTable(
-              reservoirRegionName);
-          Misc.dropReservoirRegionForSampleTable(region);
-        } else {
-          PartitionedRegion region = Misc.createReservoirRegionForSampleTable(
-              reservoirRegionName, resolvedBaseName);
-          if (Misc.initialDDLReplayDone()) {
-            Assert.assertTrue(region != null);
-          }
-        }
+        GfxdSystemProcedures.createOrDropReservoirRegion(reservoirRegionName,
+            resolvedBaseName, isDrop);
       }
 
       @Override