[geode] branch feature/GEODE-5379 updated: GEODE-5379: Do not reset affinityRetryCount after executeOnServer successfully to avoid stack overflow.

[eshu11]

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

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


The following commit(s) were added to refs/heads/feature/GEODE-5379 by this 
push:
     new 6a22a70  GEODE-5379: Do not reset affinityRetryCount after 
executeOnServer successfully to avoid stack overflow.
6a22a70 is described below

commit 6a22a704bfc56a97c15b2b9762f8080407e6d214
Author: eshu <e...@pivotal.io>
AuthorDate: Tue Jul 3 11:20:53 2018 -0700

    GEODE-5379: Do not reset affinityRetryCount after executeOnServer 
successfully to avoid stack overflow.
---
 .../cache/client/internal/OpExecutorImpl.java      |  6 +++-
 .../geode/cache/client/internal/TXFailoverOp.java  |  2 +-
 .../client/internal/OpExecutorImplJUnitTest.java   | 32 ++++++++++++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
index 4116dde..d8980a6 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
@@ -229,7 +229,6 @@ public class OpExecutorImpl implements ExecutablePool {
   private Object executeWithServerAffinity(ServerLocation loc, Op op) {
     try {
       Object retVal = executeOnServer(loc, op, true, false);
-      affinityRetryCount.set(0);
       return retVal;
     } catch (ServerConnectivityException e) {
       if (logger.isDebugEnabled()) {
@@ -295,6 +294,11 @@ public class OpExecutorImpl implements ExecutablePool {
     return this.affinityServerLocation.get();
   }
 
+  // for test only
+  protected int getAffinityRetryCount() {
+    return affinityRetryCount.get();
+  }
+
   public void setServerAffinityLocation(ServerLocation serverLocation) {
     assert this.affinityServerLocation.get() == null;
     this.affinityServerLocation.set(serverLocation);
diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java
 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java
index 90714c1..5786999 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java
@@ -33,7 +33,7 @@ public class TXFailoverOp {
     // no instance
   }
 
-  private static class TXFailoverOpImpl extends AbstractOp {
+  protected static class TXFailoverOpImpl extends AbstractOp {
     int txId;
 
     protected TXFailoverOpImpl(int txId) {
diff --git 
a/geode-core/src/test/java/org/apache/geode/cache/client/internal/OpExecutorImplJUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/cache/client/internal/OpExecutorImplJUnitTest.java
index 5478292..3af8332 100644
--- 
a/geode-core/src/test/java/org/apache/geode/cache/client/internal/OpExecutorImplJUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/cache/client/internal/OpExecutorImplJUnitTest.java
@@ -16,6 +16,11 @@ package org.apache.geode.cache.client.internal;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -42,6 +47,7 @@ import 
org.apache.geode.cache.client.internal.pooling.ConnectionManager;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.internal.cache.tier.sockets.Message;
 import org.apache.geode.internal.cache.tier.sockets.ServerQueueStatus;
 import org.apache.geode.internal.logging.InternalLogWriter;
 import org.apache.geode.internal.logging.LocalLogWriter;
@@ -458,6 +464,32 @@ public class OpExecutorImplJUnitTest {
     assertEquals(0, returns);
   }
 
+  @Test
+  public void executeWithServerAffinityDoesNotResetAffinityRetryCount() {
+    OpExecutorImpl opExecutor =
+        spy(new OpExecutorImpl(manager, queueManager, endpointManager, 
riTracker, -1,
+            10, true, cancelCriterion, mock(PoolImpl.class)));
+    opExecutor.setupServerAffinity(true);
+
+    Op txSynchronizationOp = mock(TXSynchronizationOp.Impl.class);
+    ServerLocation serverLocation = mock(ServerLocation.class);
+    ServerConnectivityException serverConnectivityException =
+        mock(ServerConnectivityException.class);
+
+    doReturn(serverLocation).when(opExecutor).getNextOpServerLocation();
+    
doThrow(serverConnectivityException).when(opExecutor).executeOnServer(serverLocation,
+        txSynchronizationOp, true, false);
+    when(((AbstractOp) 
txSynchronizationOp).getMessage()).thenReturn(mock(Message.class));
+    opExecutor.execute(txSynchronizationOp);
+    assertEquals(1, opExecutor.getAffinityRetryCount());
+
+    Op txFailoverOp = mock(TXFailoverOp.TXFailoverOpImpl.class);
+    doReturn(new Object()).when(opExecutor).executeOnServer(serverLocation, 
txFailoverOp, true,
+        false);
+    opExecutor.execute(txFailoverOp);
+    assertEquals(1, opExecutor.getAffinityRetryCount());
+  }
+
   private class DummyManager implements ConnectionManager {
 
     protected int numServers = Integer.MAX_VALUE;