Repository: geode
Updated Branches:
  refs/heads/develop 571239151 -> 66cd31a7c


GEODE-3112: Fixing improper ordering of client timeout setting

This closes #639.


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/66cd31a7
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/66cd31a7
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/66cd31a7

Branch: refs/heads/develop
Commit: 66cd31a7c280efac101c75fd438e0bf0c0476904
Parents: 5712391
Author: David Anuta <david.r.an...@gmail.com>
Authored: Mon Jul 17 15:25:35 2017 -0700
Committer: Lynn Hughes-Godfrey <lhughesgodf...@pivotal.io>
Committed: Thu Jul 20 16:04:59 2017 -0700

----------------------------------------------------------------------
 .../geode/cache/client/internal/AbstractOp.java |  2 --
 .../cache/client/internal/ConnectionImpl.java   | 13 ++++-----
 .../client/internal/ExecuteFunctionOp.java      | 12 ++++----
 .../internal/ExecuteRegionFunctionOp.java       |  2 +-
 .../client/internal/ServerRegionProxy.java      |  3 --
 .../ClientFunctionTimeoutRegressionTest.java    | 29 ++++++++++----------
 6 files changed, 28 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/66cd31a7/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
index 7af4f4f..c4035f9 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
@@ -23,10 +23,8 @@ import org.apache.logging.log4j.Logger;
 import org.apache.geode.InternalGemFireError;
 import org.apache.geode.cache.client.ServerConnectivityException;
 import org.apache.geode.cache.client.ServerOperationException;
-import org.apache.geode.distributed.internal.DistributionManager;
 import org.apache.geode.internal.HeapDataOutputStream;
 import org.apache.geode.internal.Version;
-import org.apache.geode.internal.cache.CacheServerImpl;
 import org.apache.geode.internal.cache.PutAllPartialResultException;
 import org.apache.geode.internal.cache.TXManagerImpl;
 import org.apache.geode.internal.cache.tier.MessageType;

http://git-wip-us.apache.org/repos/asf/geode/blob/66cd31a7/geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
index f71b79b..078844f 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
@@ -54,6 +54,9 @@ public class ConnectionImpl implements Connection {
 
   // TODO: DEFAULT_CLIENT_FUNCTION_TIMEOUT should be private
   public static final int DEFAULT_CLIENT_FUNCTION_TIMEOUT = 0;
+  private static final String CLIENT_FUNCTION_TIMEOUT_SYSTEM_PROPERTY =
+      DistributionConfig.GEMFIRE_PREFIX + "CLIENT_FUNCTION_TIMEOUT";
+
   private static Logger logger = LogService.getLogger();
 
   /**
@@ -62,9 +65,6 @@ public class ConnectionImpl implements Connection {
    */
   private static boolean TEST_DURABLE_CLIENT_CRASH = false;
 
-  // TODO: clientFunctionTimeout is not thread-safe and should be non-static
-  private static int clientFunctionTimeout;
-
   private Socket theSocket;
   private ByteBuffer commBuffer;
   private ByteBuffer commBufferForAsyncRead;
@@ -88,13 +88,12 @@ public class ConnectionImpl implements Connection {
 
   public ConnectionImpl(InternalDistributedSystem ds, CancelCriterion 
cancelCriterion) {
     this.ds = ds;
-    int time = Integer.getInteger(DistributionConfig.GEMFIRE_PREFIX + 
"CLIENT_FUNCTION_TIMEOUT",
-        DEFAULT_CLIENT_FUNCTION_TIMEOUT);
-    clientFunctionTimeout = time >= 0 ? time : DEFAULT_CLIENT_FUNCTION_TIMEOUT;
   }
 
   public static int getClientFunctionTimeout() {
-    return clientFunctionTimeout;
+    int time = Integer.getInteger(CLIENT_FUNCTION_TIMEOUT_SYSTEM_PROPERTY,
+        DEFAULT_CLIENT_FUNCTION_TIMEOUT);
+    return time >= 0 ? time : DEFAULT_CLIENT_FUNCTION_TIMEOUT;
   }
 
   public ServerQueueStatus connect(EndpointManager endpointManager, 
ServerLocation location,

http://git-wip-us.apache.org/repos/asf/geode/blob/66cd31a7/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionOp.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionOp.java
 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionOp.java
index 5e5a4e9..5e7da23 100755
--- 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionOp.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionOp.java
@@ -14,6 +14,12 @@
  */
 package org.apache.geode.cache.client.internal;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.logging.log4j.Logger;
+
 import org.apache.geode.InternalGemFireError;
 import org.apache.geode.cache.client.ServerConnectivityException;
 import org.apache.geode.cache.client.ServerOperationException;
@@ -36,12 +42,6 @@ import org.apache.geode.internal.cache.tier.sockets.Part;
 import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.internal.logging.LogService;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
-import org.apache.logging.log4j.Logger;
-
 /**
  * Executes the function on server (possibly without region/cache).<br>
  * Also gets the result back from the server

http://git-wip-us.apache.org/repos/asf/geode/blob/66cd31a7/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOp.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOp.java
 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOp.java
index 70f3bbf..6e11110 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOp.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOp.java
@@ -36,8 +36,8 @@ import 
org.apache.geode.internal.cache.execute.AbstractExecution;
 import org.apache.geode.internal.cache.execute.BucketMovedException;
 import org.apache.geode.internal.cache.execute.FunctionStats;
 import org.apache.geode.internal.cache.execute.InternalFunctionException;
-import org.apache.geode.internal.cache.execute.MemberMappedArgument;
 import 
org.apache.geode.internal.cache.execute.InternalFunctionInvocationTargetException;
+import org.apache.geode.internal.cache.execute.MemberMappedArgument;
 import org.apache.geode.internal.cache.execute.ServerRegionFunctionExecutor;
 import org.apache.geode.internal.cache.tier.MessageType;
 import org.apache.geode.internal.cache.tier.sockets.ChunkedMessage;

http://git-wip-us.apache.org/repos/asf/geode/blob/66cd31a7/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java
 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java
index 50eb372..7a1f160 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java
@@ -23,8 +23,6 @@ import java.util.Set;
 
 import org.apache.logging.log4j.Logger;
 
-import org.apache.geode.cache.CacheLoader;
-import org.apache.geode.cache.CacheWriter;
 import org.apache.geode.cache.DataPolicy;
 import org.apache.geode.cache.InterestResultPolicy;
 import org.apache.geode.cache.Operation;
@@ -35,7 +33,6 @@ import 
org.apache.geode.cache.client.internal.ContainsKeyOp.MODE;
 import org.apache.geode.cache.execute.Function;
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.internal.ServerLocation;
-import org.apache.geode.internal.cache.AbstractRegion;
 import org.apache.geode.internal.cache.ClientServerObserver;
 import org.apache.geode.internal.cache.ClientServerObserverHolder;
 import org.apache.geode.internal.cache.EntryEventImpl;

http://git-wip-us.apache.org/repos/asf/geode/blob/66cd31a7/geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientFunctionTimeoutRegressionTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientFunctionTimeoutRegressionTest.java
 
b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientFunctionTimeoutRegressionTest.java
index b69a6b9..6de8616 100644
--- 
a/geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientFunctionTimeoutRegressionTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientFunctionTimeoutRegressionTest.java
@@ -18,6 +18,19 @@ import static 
org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
 import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Properties;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.PartitionAttributesFactory;
 import org.apache.geode.cache.Region;
@@ -48,18 +61,6 @@ import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
 import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
 import org.apache.geode.test.junit.categories.DistributedTest;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Properties;
-import junitparams.JUnitParamsRunner;
-import junitparams.Parameters;
 
 @Category(DistributedTest.class)
 @RunWith(JUnitParamsRunner.class)
@@ -95,8 +96,8 @@ public class ClientFunctionTimeoutRegressionTest extends 
JUnit4DistributedTestCa
   }
 
   @Test
-  @Parameters({"false,0,server", "false,0,region", "true,0,region", 
"false,6000,server",
-      "false,6000,region", "true,6000,region"})
+  @Parameters({"false,0,server", "false,6000,server", "false,0,region", 
"false,6000,region",
+      "true,0,region", "true,6000,region"})
   public void testExecuteFunctionReadsDefaultTimeout(boolean createPR, int 
timeout, String mode)
       throws Exception {
     // start server

Reply via email to