geode-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (GEODE-3299) Gfsh functions should acquire Cache from FunctionContext
Date Wed, 25 Oct 2017 20:44:00 GMT

    [ https://issues.apache.org/jira/browse/GEODE-3299?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16219483#comment-16219483
] 

ASF GitHub Bot commented on GEODE-3299:
---------------------------------------

pdxrunner closed pull request #975: GEODE-3299: throw CacheClosedException in FunctionContext.getCache()
URL: https://github.com/apache/geode/pull/975
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/execute/FunctionContextImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/execute/FunctionContextImpl.java
index 5fb52fc26b..985f12e8b3 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/execute/FunctionContextImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/execute/FunctionContextImpl.java
@@ -15,6 +15,7 @@
 package org.apache.geode.internal.cache.execute;
 
 import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheClosedException;
 import org.apache.geode.cache.execute.Execution;
 import org.apache.geode.cache.execute.Function;
 import org.apache.geode.cache.execute.FunctionContext;
@@ -99,7 +100,10 @@ public boolean isPossibleDuplicate() {
   }
 
   @Override
-  public Cache getCache() {
+  public Cache getCache() throws CacheClosedException {
+    if (cache == null) {
+      throw new CacheClosedException("FunctionContext does not have a valid Cache");
+    }
     return cache;
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionDescriptionFunction.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionDescriptionFunction.java
index 11c3678b45..afaeac507c 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionDescriptionFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionDescriptionFunction.java
@@ -43,8 +43,6 @@ public void execute(FunctionContext context) {
       } else {
         context.getResultSender().lastResult(null);
       }
-    } catch (CacheClosedException e) {
-      context.getResultSender().sendException(e);
     } catch (Exception e) {
       context.getResultSender().sendException(e);
     }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ShowMissingDiskStoresFunction.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ShowMissingDiskStoresFunction.java
index 2a1b74658e..656c0fd1c3 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ShowMissingDiskStoresFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ShowMissingDiskStoresFunction.java
@@ -23,7 +23,6 @@
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.internal.InternalEntity;
-import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.PartitionedRegion;
 import org.apache.geode.internal.cache.partitioned.ColocatedRegionDetails;
@@ -65,23 +64,23 @@ public void execute(FunctionContext context) {
           }
         }
       }
+    } catch (Exception e) {
+      context.getResultSender().sendException(e);
+    }
 
-      if (memberMissingIDs.isEmpty() && missingColocatedRegions.isEmpty()) {
-        context.getResultSender().lastResult(null);
-      } else {
-        if (!memberMissingIDs.isEmpty()) {
-          if (missingColocatedRegions.isEmpty()) {
-            context.getResultSender().lastResult(memberMissingIDs);
-          } else {
-            context.getResultSender().sendResult(memberMissingIDs);
-          }
-        }
-        if (!missingColocatedRegions.isEmpty()) {
-          context.getResultSender().lastResult(missingColocatedRegions);
+    if (memberMissingIDs.isEmpty() && missingColocatedRegions.isEmpty()) {
+      context.getResultSender().lastResult(null);
+    } else {
+      if (!memberMissingIDs.isEmpty()) {
+        if (missingColocatedRegions.isEmpty()) {
+          context.getResultSender().lastResult(memberMissingIDs);
+        } else {
+          context.getResultSender().sendResult(memberMissingIDs);
         }
       }
-    } catch (Exception e) {
-      context.getResultSender().sendException(e);
+      if (!missingColocatedRegions.isEmpty()) {
+        context.getResultSender().lastResult(missingColocatedRegions);
+      }
     }
   }
 
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ShowMissingDiskStoresFunctionJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ShowMissingDiskStoresFunctionJUnitTest.java
index 7e6688a3e7..235ae39d49 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ShowMissingDiskStoresFunctionJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ShowMissingDiskStoresFunctionJUnitTest.java
@@ -19,7 +19,6 @@
 import static org.mockito.Mockito.when;
 
 import java.net.InetAddress;
-import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -30,7 +29,7 @@
 import java.util.Map;
 import java.util.Set;
 
-import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.cache.CacheClosedException;
 import org.apache.logging.log4j.core.Appender;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.Logger;
@@ -42,7 +41,6 @@
 import org.junit.rules.ExpectedException;
 import org.mockito.ArgumentCaptor;
 
-import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.PartitionAttributes;
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.ResultSender;
@@ -98,18 +96,13 @@ public void setUp() throws Exception {
   }
 
   @Test
-  public void testExecute() {
+  public void testExecute() throws Exception {
     List<?> results = null;
 
     when(cache.getPersistentMemberManager()).thenReturn(memberManager);
 
     smdsFunc.execute(context);
-    try {
-      results = resultSender.getResults();
-    } catch (Throwable e) {
-      e.printStackTrace();
-      fail("Unexpected exception");
-    }
+    results = resultSender.getResults();
     assertNotNull(results);
   }
 
@@ -118,7 +111,6 @@ public void testExecuteWithNullContextThrowsRuntimeException() {
     expectedException.expect(RuntimeException.class);
 
     smdsFunc.execute(null);
-    fail("Missing expected RuntimeException");
   }
 
   /**
@@ -126,15 +118,13 @@ public void testExecuteWithNullContextThrowsRuntimeException() {
    * {@link org.apache.geode.management.internal.cli.functions.ShowMissingDiskStoresFunction#execute(org.apache.geode.cache.execute.FunctionContext)}.
    */
   @Test
-  public void testExecuteWithNullCacheInstanceHasEmptyResults() throws Throwable {
+  public void testExecuteWithNullCacheInstanceThrowsCacheClosedException() throws Throwable
{
+    expectedException.expect(CacheClosedException.class);
     context = new FunctionContextImpl(null, "testFunction", null, resultSender);
     List<?> results = null;
 
     smdsFunc.execute(context);
     results = resultSender.getResults();
-    assertNotNull(results);
-    assertEquals(1, results.size());
-    assertNull(results.get(0));
   }
 
   @Test
@@ -195,8 +185,6 @@ public void testExecuteReturnsMissingDiskStores() throws Throwable {
         .equals("/diskStore2")) {
       assertEquals("/diskStore1",
           ((PersistentMemberPattern) detailSet.toArray()[1]).getDirectory());
-    } else {
-      fail("Incorrect missing colocated region results");
     }
   }
 
@@ -229,8 +217,6 @@ public void testExecuteReturnsMissingColocatedRegions() throws Throwable
{
       assertEquals("child2", ((ColocatedRegionDetails) detailSet.toArray()[1]).getChild());
     } else if (((ColocatedRegionDetails) detailSet.toArray()[0]).getChild().equals("child2"))
{
       assertEquals("child1", ((ColocatedRegionDetails) detailSet.toArray()[1]).getChild());
-    } else {
-      fail("Incorrect missing colocated region results");
     }
   }
 
@@ -277,8 +263,6 @@ public void testExecuteReturnsMissingStoresAndRegions() throws Throwable
{
             .equals("/diskStore2")) {
           assertEquals("/diskStore1",
               ((PersistentMemberPattern) detailSet.toArray()[1]).getDirectory());
-        } else {
-          fail("Incorrect missing colocated region results");
         }
       } else if (detailSet.toArray()[0] instanceof ColocatedRegionDetails) {
         assertEquals(2, detailSet.toArray().length);
@@ -293,8 +277,6 @@ public void testExecuteReturnsMissingStoresAndRegions() throws Throwable
{
         } else {
           fail("Incorrect missing colocated region results");
         }
-      } else {
-        fail("Unexpected result type: " + detailSet.toArray()[0].getClass());
       }
     }
   }
@@ -307,7 +289,6 @@ public void testExecuteCatchesExceptions() throws Exception {
 
     smdsFunc.execute(context);
     List<?> results = resultSender.getResults();
-    fail("Failed to catch expected RuntimeException");
   }
 
   @Test


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Gfsh functions should acquire Cache from FunctionContext
> --------------------------------------------------------
>
>                 Key: GEODE-3299
>                 URL: https://issues.apache.org/jira/browse/GEODE-3299
>             Project: Geode
>          Issue Type: Bug
>          Components: gfsh
>            Reporter: Kirk Lund
>            Assignee: Kenneth Howe
>             Fix For: 1.4.0
>
>
> Gfsh functions in geode-core org.apache.geode.management.internal.cli.functions currently
acquire a reference to the Cache in several ways including from a singleton in CacheFactory
which is prone to deadlocking. They should be modified to uniformly acquire the Cache from
FunctionContext instead.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message