From 8133cd5305d7913453abb2d48da12f672c0ce334 Mon Sep 17 00:00:00 2001
From: Inigo Goiri <inigoiri@apache.org>
Date: Fri, 9 Mar 2018 18:25:05 -0800
Subject: [PATCH] HDFS-13232. RBF: ConnectionPool should return first usable
 connection. Contributed by Ekanth S.

---
 .../federation/router/ConnectionPool.java     |  2 +-
 .../router/TestConnectionManager.java         | 43 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java
index 5c77c5910e4..5af8a867903 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java
@@ -159,7 +159,7 @@ public class ConnectionPool {
     for (int i=0; i<size; i++) {
       int index = (threadIndex + i) % size;
       conn = tmpConnections.get(index);
-      if (conn != null && !conn.isUsable()) {
+      if (conn != null && conn.isUsable()) {
         return conn;
       }
     }
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestConnectionManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestConnectionManager.java
index fe9f1955ed1..741d1f6a203 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestConnectionManager.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestConnectionManager.java
@@ -28,6 +28,7 @@ import java.io.IOException;
 import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Test functionalities of {@link ConnectionManager}, which manages a pool
@@ -41,6 +42,8 @@ public class TestConnectionManager {
       UserGroupInformation.createUserForTesting("user1", TEST_GROUP);
   private static final UserGroupInformation TEST_USER2 =
       UserGroupInformation.createUserForTesting("user2", TEST_GROUP);
+  private static final UserGroupInformation TEST_USER3 =
+      UserGroupInformation.createUserForTesting("user3", TEST_GROUP);
   private static final String TEST_NN_ADDRESS = "nn1:8080";
 
   @Before
@@ -87,6 +90,46 @@ public class TestConnectionManager {
     connManager.cleanup(pool1);
     checkPoolConnections(TEST_USER1, 8, 4);
     checkPoolConnections(TEST_USER2, 10, 10);
+
+    // Make sure the number of connections doesn't go below minSize
+    ConnectionPool pool3 = new ConnectionPool(
+        conf, TEST_NN_ADDRESS, TEST_USER3, 2, 10);
+    addConnectionsToPool(pool3, 10, 0);
+    poolMap.put(new ConnectionPoolId(TEST_USER2, TEST_NN_ADDRESS), pool3);
+    connManager.cleanup(pool3);
+    checkPoolConnections(TEST_USER3, 2, 0);
+    // With active connections added to pool, make sure it honors the
+    // MIN_ACTIVE_RATIO again
+    addConnectionsToPool(pool3, 10, 2);
+    connManager.cleanup(pool3);
+    checkPoolConnections(TEST_USER3, 4, 2);
+  }
+
+  @Test
+  public void testGetConnection() throws Exception {
+    Map<ConnectionPoolId, ConnectionPool> poolMap = connManager.getPools();
+    final int totalConns = 10;
+    int activeConns = 5;
+
+    ConnectionPool pool = new ConnectionPool(
+        conf, TEST_NN_ADDRESS, TEST_USER1, 0, 10);
+    addConnectionsToPool(pool, totalConns, activeConns);
+    poolMap.put(new ConnectionPoolId(TEST_USER1, TEST_NN_ADDRESS), pool);
+
+    // All remaining connections should be usable
+    final int remainingSlots = totalConns - activeConns;
+    for (int i = 0; i < remainingSlots; i++) {
+      ConnectionContext cc = pool.getConnection();
+      assertTrue(cc.isUsable());
+      cc.getClient();
+      activeConns++;
+    }
+
+    checkPoolConnections(TEST_USER1, totalConns, activeConns);
+
+    // Ask for more and this returns an active connection
+    ConnectionContext cc = pool.getConnection();
+    assertTrue(cc.isActive());
   }
 
   private void addConnectionsToPool(ConnectionPool pool, int numTotalConn,