Mailing-List: contact commits-help@geode.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geode.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: upthewaterspout@apache.org
To: commits@geode.incubator.apache.org
Date: Thu, 11 Aug 2016 21:02:17 -0000
Message-Id: <2d40ee0ab96248fca6a38719b222548a@git.apache.org>
In-Reply-To: <78a88380c2994ddb927007a1c6c035d9@git.apache.org>
References: <78a88380c2994ddb927007a1c6c035d9@git.apache.org>
Subject: [2/2] incubator-geode git commit: GEODE-1583: Pass the keep alive
 flag when destroying a connection
archived-at: Thu, 11 Aug 2016 21:04:23 -0000

GEODE-1583: Pass the keep alive flag when destroying a connection

The queue manager was closing connections without sending the
CloseConnectionOp to the server. Changing the code to call close instead
of destroy and passing the keepAlive flag along with the close. Also,
changing the cache to set keepAlive *before* marking the cache as
closed.

This should prevent any races where cache closure triggers the cache
client updater to destroy connections or close them with the wrong
keepAlive setting while the cache is being closed.


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

Branch: refs/heads/develop
Commit: dba71fd5e58eb3ca75da87f40e7b5d9d69943a7b
Parents: 083160e
Author: Dan Smith <upthewaterspout@apache.org>
Authored: Mon Jul 25 17:29:30 2016 -0700
Committer: Dan Smith <upthewaterspout@apache.org>
Committed: Thu Aug 11 13:53:39 2016 -0700

----------------------------------------------------------------------
 .../gemfire/cache/client/internal/InternalPool.java   |  1 +
 .../gemfire/cache/client/internal/PoolImpl.java       | 14 ++++++++++----
 .../cache/client/internal/QueueManagerImpl.java       | 14 +++++++++++---
 .../gemfire/internal/cache/GemFireCacheImpl.java      |  2 +-
 .../internal/AutoConnectionSourceImplJUnitTest.java   | 11 ++++++++---
 .../cache/client/internal/QueueManagerJUnitTest.java  |  6 ++++++
 6 files changed, 37 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/dba71fd5/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/InternalPool.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/InternalPool.java b/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/InternalPool.java
index 8c04525..fa27d99 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/InternalPool.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/InternalPool.java
@@ -39,4 +39,5 @@ public interface InternalPool extends Pool, ExecutablePool {
   boolean isDurableClient();
   void detach();
   String getPoolOrCacheCancelInProgress();
+  boolean getKeepAlive();
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/dba71fd5/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/PoolImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/PoolImpl.java b/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/PoolImpl.java
index 3844dbb..0563456 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/PoolImpl.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/PoolImpl.java
@@ -445,10 +445,6 @@ public class PoolImpl implements InternalPool {
     return sb.toString();
   }
 
-  public boolean getKeepAlive(){
-      return this.keepAlive;
-  }
-  
   public void destroy(boolean keepAlive) {
     int cnt = getAttachCount();
     this.keepAlive = keepAlive;
@@ -511,6 +507,7 @@ public class PoolImpl implements InternalPool {
   public synchronized void basicDestroy(boolean keepAlive) {
     if (!isDestroyed()) {
       this.destroyed = true;
+      this.keepAlive = keepAlive;
       // LOG: changed from config to info
       logger.info(LocalizedMessage.create(LocalizedStrings.PoolImpl_DESTROYING_CONNECTION_POOL_0, name));
 
@@ -1420,6 +1417,15 @@ public class PoolImpl implements InternalPool {
     } 
   }
 
+  public boolean getKeepAlive() {
+    GemFireCacheImpl cache = GemFireCacheImpl.getInstance();
+    if(cache == null) {
+      return keepAlive;
+    }
+
+    return cache.keepDurableSubscriptionsAlive();
+  }
+
   public ArrayList<ProxyCache> getProxyCacheList() {
     return this.proxyCacheList;
   }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/dba71fd5/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/QueueManagerImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/QueueManagerImpl.java b/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/QueueManagerImpl.java
index 3065691..38ca6b1 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/QueueManagerImpl.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/QueueManagerImpl.java
@@ -416,8 +416,12 @@ public class QueueManagerImpl implements QueueManager {
       //if same client updater then only remove as we don't know whether it has created new updater/connection on same endpoint or not..
       deadConnection = queueConnections.getConnection(endpoint);
       if (deadConnection != null && ccu.equals(deadConnection.getUpdater())) {
-        queueConnections = queueConnections.removeConnection(deadConnection);        
-        deadConnection.internalDestroy();
+        queueConnections = queueConnections.removeConnection(deadConnection);
+        try {
+          deadConnection.internalClose(pool.getKeepAlive());
+        }  catch(Exception e) {
+          logger.warn("Error destroying client to server connection to {}", deadConnection.getEndpoint(), e);
+        }
       }      
     }    
     
@@ -1002,7 +1006,11 @@ public class QueueManagerImpl implements QueueManager {
       if(logger.isDebugEnabled()) {
         logger.debug("Endpoint {} crashed while creating a connection. The connection will be destroyed", connection.getEndpoint());
       }
-      connection.internalDestroy();
+      try {
+        connection.internalClose(pool.getKeepAlive());
+      }  catch(Exception e) {
+        logger.warn("Error destroying client to server connection to {}", connection.getEndpoint(), e);
+      }
     }
     
     return !isBadConnection;

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/dba71fd5/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
index 84ba926..76a7bad 100755
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
@@ -1983,6 +1983,7 @@ public class GemFireCacheImpl implements InternalCache, ClientCache, HasCachePer
         }
       }
 
+      this.keepAlive = keepalive;
       isClosing = true;
       logger.info(LocalizedMessage.create(LocalizedStrings.GemFireCache_0_NOW_CLOSING, this));
 
@@ -1999,7 +2000,6 @@ public class GemFireCacheImpl implements InternalCache, ClientCache, HasCachePer
       }
       TXStateProxy tx = null;
       try {
-        this.keepAlive = keepalive;
 
         if (this.txMgr != null) {
           tx = this.txMgr.internalSuspend();

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/dba71fd5/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/AutoConnectionSourceImplJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
index ef23d00..42bd683 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
@@ -228,9 +228,14 @@ public class AutoConnectionSourceImplJUnitTest {
   }
   
   public class FakePool implements InternalPool {
-      public String getPoolOrCacheCancelInProgress() {return null; } 
-      
-      public EndpointManager getEndpointManager() {
+      public String getPoolOrCacheCancelInProgress() {return null; }
+
+    @Override
+    public boolean getKeepAlive() {
+      return false;
+    }
+
+    public EndpointManager getEndpointManager() {
         return null;
       }
 

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/dba71fd5/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/QueueManagerJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/QueueManagerJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/QueueManagerJUnitTest.java
index 79f922a..668f041 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/QueueManagerJUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/QueueManagerJUnitTest.java
@@ -292,6 +292,12 @@ public class QueueManagerJUnitTest {
   private class DummyPool implements InternalPool {
     
     public String getPoolOrCacheCancelInProgress() {return null; }
+
+    @Override
+    public boolean getKeepAlive() {
+      return false;
+    }
+
     public Object execute(Op op, int retryAttempts) {
       return null;
     }