http-client-branch: add a property to make it possible to limit the size of the HTTP/1.1 coonection pool http-client-branch
authordfuchs
Wed, 09 May 2018 16:45:54 -0700
branchhttp-client-branch
changeset 56538 9bdcfc7d2b9c
parent 56537 5665667a080c
child 56539 a738880f0bd8
http-client-branch: add a property to make it possible to limit the size of the HTTP/1.1 coonection pool
src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java
test/jdk/java/net/httpclient/HandshakeFailureTest.java
test/jdk/java/net/httpclient/whitebox/ConnectionPoolTestDriver.java
test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/ConnectionPoolTest.java
--- a/src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java	Mon May 07 14:48:59 2018 -0700
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java	Wed May 09 16:45:54 2018 -0700
@@ -53,6 +53,8 @@
 
     static final long KEEP_ALIVE = Utils.getIntegerNetProperty(
             "jdk.httpclient.keepalive.timeout", 1200); // seconds
+    static final long MAX_POOL_SIZE = Utils.getIntegerNetProperty(
+            "jdk.httpclient.connectionPoolSize", 0); // unbounded
     final Logger debug = Utils.getDebugLogger(this::dbgString, Utils.DEBUG);
 
     // Pools of idle connections
@@ -160,6 +162,7 @@
         CleanupTrigger cleanup = registerCleanupTrigger(conn);
 
         // it's possible that cleanup may have been called.
+        HttpConnection toClose = null;
         synchronized(this) {
             if (cleanup.isDone()) {
                 return;
@@ -167,6 +170,10 @@
                 conn.close();
                 return;
             }
+            if (MAX_POOL_SIZE > 0 && expiryList.size() >= MAX_POOL_SIZE) {
+                toClose = expiryList.removeOldest();
+                if (toClose != null) removeFromPool(toClose);
+            }
             if (conn instanceof PlainHttpConnection) {
                 putConnection(conn, plainPool);
             } else {
@@ -175,6 +182,13 @@
             }
             expiryList.add(conn, now, keepAlive);
         }
+        if (toClose != null) {
+            if (debug.on()) {
+                debug.log("Maximum pool size reached: removing oldest connection %s",
+                          toClose.dbgString());
+            }
+            close(toClose);
+        }
         //System.out.println("Return to pool: " + conn);
     }
 
@@ -314,6 +328,8 @@
         private final LinkedList<ExpiryEntry> list = new LinkedList<>();
         private volatile boolean mayContainEntries;
 
+        int size() { return list.size(); }
+
         // A loosely accurate boolean whose value is computed
         // at the end of each operation performed on ExpiryList;
         // Does not require synchronizing on the ConnectionPool.
@@ -331,6 +347,13 @@
 
         // should only be called while holding a synchronization
         // lock on the ConnectionPool
+        HttpConnection removeOldest() {
+            ExpiryEntry entry = list.pollLast();
+            return entry == null ? null : entry.connection;
+        }
+
+        // should only be called while holding a synchronization
+        // lock on the ConnectionPool
         void add(HttpConnection conn) {
             add(conn, Instant.now(), KEEP_ALIVE);
         }
@@ -419,17 +442,38 @@
         }
     }
 
+    // Remove a connection from the pool.
+    // should only be called while holding a synchronization
+    // lock on the ConnectionPool
+    private void removeFromPool(HttpConnection c) {
+        assert Thread.holdsLock(this);
+        if (c instanceof PlainHttpConnection) {
+            removeFromPool(c, plainPool);
+        } else {
+            assert c.isSecure();
+            removeFromPool(c, sslPool);
+        }
+    }
+
+    // Used by tests
+    synchronized boolean contains(HttpConnection c) {
+        final CacheKey key = c.cacheKey();
+        List<HttpConnection> list;
+        if ((list = plainPool.get(key)) != null) {
+            if (list.contains(c)) return true;
+        }
+        if ((list = sslPool.get(key)) != null) {
+            if (list.contains(c)) return true;
+        }
+        return false;
+    }
+
     void cleanup(HttpConnection c, Throwable error) {
         if (debug.on())
             debug.log("%s : ConnectionPool.cleanup(%s)",
                     String.valueOf(c.getConnectionFlow()), error);
         synchronized(this) {
-            if (c instanceof PlainHttpConnection) {
-                removeFromPool(c, plainPool);
-            } else {
-                assert c.isSecure();
-                removeFromPool(c, sslPool);
-            }
+            removeFromPool(c);
             expiryList.remove(c);
         }
         c.close();
--- a/test/jdk/java/net/httpclient/HandshakeFailureTest.java	Mon May 07 14:48:59 2018 -0700
+++ b/test/jdk/java/net/httpclient/HandshakeFailureTest.java	Wed May 09 16:45:54 2018 -0700
@@ -45,7 +45,7 @@
 
 /**
  * @test
- * @run main/othervm HandshakeFailureTest
+ * @run main/othervm -Djdk.internal.httpclient.debug=true HandshakeFailureTest
  * @summary Verify SSLHandshakeException is received when the handshake fails,
  * either because the server closes ( EOF ) the connection during handshaking
  * or no cipher suite ( or similar ) can be negotiated.
--- a/test/jdk/java/net/httpclient/whitebox/ConnectionPoolTestDriver.java	Mon May 07 14:48:59 2018 -0700
+++ b/test/jdk/java/net/httpclient/whitebox/ConnectionPoolTestDriver.java	Wed May 09 16:45:54 2018 -0700
@@ -31,5 +31,8 @@
   *         java.management
  * @run main/othervm
  *       --add-reads java.net.http=java.management
- *       java.net.http/jdk.internal.net.http.ConnectionPoolTest
+ *       java.net.http/jdk.internal.net.http.ConnectionPoolTest testCacheCleaners
+ * @run main/othervm
+ *       --add-reads java.net.http=java.management
+ *       java.net.http/jdk.internal.net.http.ConnectionPoolTest testPoolSize
  */
--- a/test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/ConnectionPoolTest.java	Mon May 07 14:48:59 2018 -0700
+++ b/test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/ConnectionPoolTest.java	Wed May 09 16:45:54 2018 -0700
@@ -31,6 +31,7 @@
 import java.net.ProxySelector;
 import java.nio.ByteBuffer;
 import java.nio.channels.SocketChannel;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 import java.util.Random;
@@ -67,14 +68,24 @@
     }
 
     public static void main(String[] args) throws Exception {
-        testCacheCleaners();
+        if (args.length == 0) {
+            args = new String[] {"testCacheCleaners"};
+        }
+        for (String arg : args) {
+            if ("testCacheCleaners".equals(arg)) {
+                testCacheCleaners();
+            } else if ("testPoolSize".equals(arg)) {
+                assert args.length == 1 : "testPoolSize should be run in its own VM";
+                testPoolSize();
+            }
+        }
     }
 
     public static void testCacheCleaners() throws Exception {
         ConnectionPool pool = new ConnectionPool(666);
         HttpClient client = new HttpClientStub(pool);
         InetSocketAddress proxy = InetSocketAddress.createUnresolved("bar", 80);
-        System.out.println("Adding 10 connections to pool");
+        System.out.println("Adding 20 connections to pool");
         Random random = new Random();
 
         final int count = 20;
@@ -146,6 +157,74 @@
         }
     }
 
+    public static void testPoolSize() throws Exception {
+        final int MAX_POOL_SIZE = 10;
+        System.setProperty("jdk.httpclient.connectionPoolSize",
+                String.valueOf(MAX_POOL_SIZE));
+        ConnectionPool pool = new ConnectionPool(666);
+        HttpClient client = new HttpClientStub(pool);
+        InetSocketAddress proxy = InetSocketAddress.createUnresolved("bar", 80);
+        System.out.println("Adding 20 connections to pool");
+        Random random = new Random();
+
+        final int count = 20;
+        Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS);
+        int[] keepAlives = new int[count];
+        HttpConnectionStub[] connections = new HttpConnectionStub[count];
+        long purge = pool.purgeExpiredConnectionsAndReturnNextDeadline(now);
+        long expected = 0;
+        if (purge != expected) {
+            throw new RuntimeException("Bad purge delay: " + purge
+                    + ", expected " + expected);
+        }
+        expected = Long.MAX_VALUE;
+        int previous = 0;
+        for (int i=0; i<count; i++) {
+            InetSocketAddress addr = InetSocketAddress.createUnresolved("foo"+i, 80);
+            keepAlives[i] = random.nextInt(10) * 10  + 5 + previous;
+            previous = keepAlives[i];
+            connections[i] = new HttpConnectionStub(client, addr, proxy, true);
+            System.out.println("Adding connection: " + now
+                    + " keepAlive: " + keepAlives[i]
+                    + " /" + connections[i]);
+            pool.returnToPool(connections[i], now, keepAlives[i]);
+            if (i < MAX_POOL_SIZE) {
+                expected = Math.min(expected, keepAlives[i] * 1000);
+            } else {
+                expected = keepAlives[i-MAX_POOL_SIZE+1] * 1000;
+                if (pool.contains(connections[i-MAX_POOL_SIZE])) {
+                    throw new RuntimeException("Connection[" + i + "]/"
+                            + connections[i] + " should have been removed");
+                }
+            }
+            purge = pool.purgeExpiredConnectionsAndReturnNextDeadline(now);
+            if (purge != expected) {
+                throw new RuntimeException("Bad purge delay for " + i + ": "
+                        + purge + ", expected " + expected);
+            }
+        }
+
+        long opened = java.util.stream.Stream.of(connections)
+                .filter(HttpConnectionStub::connected).count();
+        if (opened != MAX_POOL_SIZE) {
+            throw new RuntimeException("Opened: expected "
+                    + count + " got " + opened);
+        }
+        for (int i=0 ; i<count; i++) {
+            boolean closed = (i < count - MAX_POOL_SIZE);
+            if (connections[i].closed != closed) {
+                throw new RuntimeException("connection[" + i + "] should be "
+                        + (closed ? "closed" : "opened"));
+            }
+            if (pool.contains(connections[i]) == closed) {
+                throw new RuntimeException("Connection[" + i + "]/"
+                        + connections[i] + " should "
+                        + (closed ? "" : "not ")
+                        + "have been removed");
+            }
+        }
+    }
+
     static <T> T error() {
         throw new InternalError("Should not reach here: wrong test assumptions!");
     }