phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gjac...@apache.org
Subject phoenix git commit: PHOENIX-3877 - Connection throttling doesn't always decrement on connection close
Date Tue, 23 May 2017 23:45:27 GMT
Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-1.1 a96fafcea -> e2a19898f


PHOENIX-3877 - Connection throttling doesn't always decrement on connection close


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

Branch: refs/heads/4.x-HBase-1.1
Commit: e2a19898f52cafcb541169cc143ca89e8ee36dad
Parents: a96fafc
Author: gjacoby <gjacoby@salesforce.com>
Authored: Tue May 23 10:02:54 2017 -0700
Committer: gjacoby <gjacoby@apache.org>
Committed: Tue May 23 16:45:20 2017 -0700

----------------------------------------------------------------------
 .../phoenix/monitoring/PhoenixMetricsIT.java    | 28 +++++++++++++++++---
 .../query/ConnectionQueryServicesImpl.java      | 11 +++++++-
 2 files changed, 34 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/e2a19898/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
index 04d125a..2838f04 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
@@ -36,6 +36,7 @@ import static org.apache.phoenix.util.PhoenixRuntime.TENANT_ID_ATTRIB;
 import static org.apache.phoenix.util.PhoenixRuntime.UPSERT_BATCH_SIZE_ATTRIB;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.sql.Connection;
 import java.sql.DriverManager;
@@ -856,14 +857,15 @@ public class PhoenixMetricsIT extends BaseUniqueNamesOwnClusterIT {
 
     @Test
     public void testGetConnectionsThrottledForSameUrl() throws Exception {
-        int expectedPhoenixConnections = 11;
+        int attemptedPhoenixConnections = 11;
+        int maxConnections = attemptedPhoenixConnections -1;
         List<Connection> connections = Lists.newArrayList();
         String zkQuorum = "localhost:" + getUtility().getZkCluster().getClientPort();
         String url = PhoenixRuntime.JDBC_PROTOCOL + PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR
+ zkQuorum +
         ':' +  CUSTOM_URL_STRING + '=' + "throttletest";
 
         Properties props = new Properties();
-        props.setProperty(QueryServices.CLIENT_CONNECTION_MAX_ALLOWED_CONNECTIONS, "10");
+        props.setProperty(QueryServices.CLIENT_CONNECTION_MAX_ALLOWED_CONNECTIONS, Integer.toString(maxConnections));
 
         GLOBAL_HCONNECTIONS_COUNTER.getMetric().reset();
         GLOBAL_QUERY_SERVICES_COUNTER.getMetric().reset();
@@ -871,7 +873,7 @@ public class PhoenixMetricsIT extends BaseUniqueNamesOwnClusterIT {
         GLOBAL_PHOENIX_CONNECTIONS_THROTTLED_COUNTER.getMetric().reset();
         boolean wasThrottled = false;
         try {
-            for (int k = 0; k < expectedPhoenixConnections; k++) {
+            for (int k = 0; k < attemptedPhoenixConnections; k++) {
                 connections.add(DriverManager.getConnection(url, props));
             }
         } catch (SQLException se) {
@@ -885,7 +887,25 @@ public class PhoenixMetricsIT extends BaseUniqueNamesOwnClusterIT {
         assertEquals(1, GLOBAL_QUERY_SERVICES_COUNTER.getMetric().getValue());
         assertTrue("No connection was throttled!", wasThrottled);
         assertEquals(1, GLOBAL_PHOENIX_CONNECTIONS_THROTTLED_COUNTER.getMetric().getValue());
-        assertEquals(expectedPhoenixConnections, GLOBAL_PHOENIX_CONNECTIONS_ATTEMPTED_COUNTER.getMetric().getValue());
+        assertEquals(maxConnections, connections.size());
+        assertTrue("Not all connections were attempted!",
+            attemptedPhoenixConnections <= GLOBAL_PHOENIX_CONNECTIONS_ATTEMPTED_COUNTER.getMetric().getValue());
+        connections.clear();
+        //now check that we decremented the counter for the connections we just released
+        try {
+            for (int k = 0; k < maxConnections; k++){
+                connections.add(DriverManager.getConnection(url, props));
+            }
+        } catch(SQLException se) {
+            if (se.getErrorCode() == (SQLExceptionCode.NEW_CONNECTION_THROTTLED).getErrorCode()){
+                fail("Connection was throttled when it shouldn't be!");
+            }
+        } finally {
+            for (Connection c : connections) {
+                c.close();
+            }
+        }
+        assertEquals(maxConnections, connections.size());
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e2a19898/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index 97f431f..7a6d344 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -3860,12 +3860,15 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices
implement
         if (returnSequenceValues) {
             ConcurrentMap<SequenceKey,Sequence> formerSequenceMap = null;
             synchronized (connectionCountLock) {
-                if (--connectionCount == 0) {
+                if (--connectionCount <= 0) {
                     if (!this.sequenceMap.isEmpty()) {
                         formerSequenceMap = this.sequenceMap;
                         this.sequenceMap = Maps.newConcurrentMap();
                     }
                 }
+                if (connectionCount < 0) {
+                    connectionCount = 0;
+                }
             }
             // Since we're using the former sequenceMap, we can do this outside
             // the lock.
@@ -3873,6 +3876,12 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices
implement
                 // When there are no more connections, attempt to return any sequences
                 returnAllSequences(formerSequenceMap);
             }
+        } else if (shouldThrottleNumConnections){ //still need to decrement connection count
+            synchronized (connectionCountLock) {
+                if (connectionCount > 0) {
+                    --connectionCount;
+                }
+            }
         }
     }
 


Mime
View raw message