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:01:33 GMT
Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-0.98 ca20d3a28 -> f57a97a08


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/f57a97a0
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/f57a97a0
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/f57a97a0

Branch: refs/heads/4.x-HBase-0.98
Commit: f57a97a081d9620dc4b366a489d5f35e6f45d121
Parents: ca20d3a
Author: gjacoby <gjacoby@salesforce.com>
Authored: Tue May 23 10:02:54 2017 -0700
Committer: gjacoby <gjacoby@apache.org>
Committed: Tue May 23 15:57:53 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/f57a97a0/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/f57a97a0/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 0da67ad..694207e 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
@@ -3859,12 +3859,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.
@@ -3872,6 +3875,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