diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java index 8ace5cc2ae6..3149176eea1 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java @@ -68,7 +68,7 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable CompletableFuture[] futures = new CompletableFuture[connectionCount]; for (int i = 0; i < connectionCount; i++) { - futures[i] = tryCreateReturningFuture(pool.getMaxEntries()); + futures[i] = tryCreateAsync(getMaxConnectionCount()); } return CompletableFuture.allOf(futures); } @@ -138,6 +138,8 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable @Override public Connection acquire(boolean create) { + if (LOG.isDebugEnabled()) + LOG.debug("Acquiring create={} on {}", create, this); Connection connection = activate(); if (connection == null && create) { @@ -159,22 +161,21 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable */ protected void tryCreate(int maxPending) { - tryCreateReturningFuture(maxPending); + tryCreateAsync(maxPending); } - private CompletableFuture tryCreateReturningFuture(int maxPending) + private CompletableFuture tryCreateAsync(int maxPending) { + int connectionCount = getConnectionCount(); if (LOG.isDebugEnabled()) - { - LOG.debug("tryCreate {}/{} connections {}/{} pending", pool.size(), pool.getMaxEntries(), getPendingConnectionCount(), maxPending); - } + LOG.debug("Try creating connection {}/{} with {}/{} pending", connectionCount, getMaxConnectionCount(), getPendingConnectionCount(), maxPending); Pool.Entry entry = pool.reserve(maxPending); if (entry == null) return CompletableFuture.completedFuture(null); if (LOG.isDebugEnabled()) - LOG.debug("newConnection {}/{} connections {}/{} pending", pool.size(), pool.getMaxEntries(), getPendingConnectionCount(), maxPending); + LOG.debug("Creating connection {}/{}", connectionCount, getMaxConnectionCount()); CompletableFuture future = new CompletableFuture<>(); destination.newConnection(new Promise<>() @@ -183,7 +184,7 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable public void succeeded(Connection connection) { if (LOG.isDebugEnabled()) - LOG.debug("Connection {}/{} creation succeeded {}", pool.size(), pool.getMaxEntries(), connection); + LOG.debug("Connection {}/{} creation succeeded {}", connectionCount, getMaxConnectionCount(), connection); if (!(connection instanceof Attachable)) { failed(new IllegalArgumentException("Invalid connection object: " + connection)); @@ -201,7 +202,7 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable public void failed(Throwable x) { if (LOG.isDebugEnabled()) - LOG.debug("Connection " + pool.size() + "/" + pool.getMaxEntries() + " creation failed", x); + LOG.debug("Connection {}/{} creation failed", connectionCount, getMaxConnectionCount(), x); entry.remove(); future.completeExceptionally(x); requester.failed(x); @@ -240,7 +241,7 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable if (entry != null) { if (LOG.isDebugEnabled()) - LOG.debug("activated {}", entry); + LOG.debug("Activated {} {}", entry, pool); Connection connection = entry.getPooled(); acquired(connection); return connection; @@ -258,8 +259,6 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable Pool.Entry entry = (Pool.Entry)attachable.getAttachment(); if (entry == null) return false; - if (LOG.isDebugEnabled()) - LOG.debug("isActive {}", entry); return !entry.isIdle(); } @@ -283,7 +282,7 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable return true; boolean reusable = pool.release(entry); if (LOG.isDebugEnabled()) - LOG.debug("Released ({}) {}", reusable, entry); + LOG.debug("Released ({}) {} {}", reusable, entry, pool); if (reusable) return true; remove(connection); @@ -308,7 +307,7 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable attachable.setAttachment(null); boolean removed = pool.remove(entry); if (LOG.isDebugEnabled()) - LOG.debug("Removed ({}) {}", removed, entry); + LOG.debug("Removed ({}) {} {}", removed, entry, pool); if (removed || force) { released(connection); diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java index 175f397075d..b2e58c80220 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java @@ -370,7 +370,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest { // Aggressively send other queued requests // in case connections are multiplexed. - return getHttpExchanges().size() > 0 ? ProcessResult.CONTINUE : ProcessResult.FINISH; + return getQueuedRequestCount() > 0 ? ProcessResult.CONTINUE : ProcessResult.FINISH; } if (LOG.isDebugEnabled()) @@ -427,7 +427,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest { if (connectionPool.isActive(connection)) { - // trigger the next request after releasing the connection + // Trigger the next request after releasing the connection. if (connectionPool.release(connection)) send(false); else diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolTest.java index ee687a23bdf..476458e739b 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolTest.java @@ -50,7 +50,6 @@ import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -65,10 +64,15 @@ public class ConnectionPoolTest private HttpClient client; public static Stream pools() + { + return Stream.concat(poolsNoRoundRobin(), + Stream.of(new ConnectionPoolFactory("round-robin", destination -> new RoundRobinConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination)))); + } + + public static Stream poolsNoRoundRobin() { return Stream.of( new ConnectionPoolFactory("duplex", destination -> new DuplexConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination)), - new ConnectionPoolFactory("round-robin", destination -> new RoundRobinConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination)), new ConnectionPoolFactory("multiplex", destination -> new MultiplexConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination, 1)) ); } @@ -301,11 +305,11 @@ public class ConnectionPoolTest } @ParameterizedTest - @MethodSource("pools") + @MethodSource("poolsNoRoundRobin") public void testConcurrentRequestsDontOpenTooManyConnections(ConnectionPoolFactory factory) throws Exception { - // Round robin connection pool does open a few more connections than expected. - Assumptions.assumeFalse(factory.name.equals("round-robin")); + // Round robin connection pool does open a few more + // connections than expected, exclude it from this test. startServer(new EmptyServerHandler()); diff --git a/jetty-jspc-maven-plugin/pom.xml b/jetty-jspc-maven-plugin/pom.xml index e34e5801f54..131d264d047 100644 --- a/jetty-jspc-maven-plugin/pom.xml +++ b/jetty-jspc-maven-plugin/pom.xml @@ -109,7 +109,7 @@ org.apache.ant ant - 1.8.4 + 1.10.8 diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java index e638fdaf713..98e99d14104 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java @@ -362,7 +362,12 @@ public class Pool implements AutoCloseable, Dumpable @Override public String toString() { - return getClass().getSimpleName() + " size=" + sharedList.size() + " closed=" + closed + " entries=" + sharedList; + return String.format("%s@%x[size=%d closed=%s entries=%s]", + getClass().getSimpleName(), + hashCode(), + sharedList.size(), + closed, + sharedList); } public class Entry @@ -544,11 +549,13 @@ public class Pool implements AutoCloseable, Dumpable public String toString() { long encoded = state.get(); - return String.format("%s@%x{usage=%d,multiplex=%d,pooled=%s}", + return String.format("%s@%x{usage=%d/%d,multiplex=%d/%d,pooled=%s}", getClass().getSimpleName(), hashCode(), AtomicBiInteger.getHi(encoded), + getMaxUsageCount(), AtomicBiInteger.getLo(encoded), + getMaxMultiplex(), pooled); } } diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/RoundRobinConnectionPoolTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/RoundRobinConnectionPoolTest.java index 3aebdde260e..81b7e1abfa8 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/RoundRobinConnectionPoolTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/RoundRobinConnectionPoolTest.java @@ -19,8 +19,9 @@ package org.eclipse.jetty.http.client; import java.io.IOException; -import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.TimeUnit; @@ -56,7 +57,7 @@ public class RoundRobinConnectionPoolTest extends AbstractTest remotePorts = new ArrayList<>(); + List remotePorts = new CopyOnWriteArrayList<>(); scenario.start(new EmptyServerHandler() { @Override @@ -115,8 +116,7 @@ public class RoundRobinConnectionPoolTest extends AbstractTest remotePorts = new ArrayList<>(); + List remotePorts = new CopyOnWriteArrayList<>(); AtomicReference requestLatch = new AtomicReference<>(); CountDownLatch serverLatch = new CountDownLatch(count); CyclicBarrier barrier = new CyclicBarrier(count + 1); @@ -127,13 +127,10 @@ public class RoundRobinConnectionPoolTest extends AbstractTest new RoundRobinConnectionPool(destination, maxConnections, destination, maxMultiplex)); - // Prime the connections, so that they are all opened - // before we actually test the round robin behavior. - for (int i = 0; i < maxConnections; ++i) - { - ContentResponse response = scenario.client.newRequest(scenario.newURI()) - .timeout(5, TimeUnit.SECONDS) - .send(); - assertEquals(HttpStatus.OK_200, response.getStatus()); - } + // Do not prime the connections, to see if the behavior is + // correct even if the connections are not pre-created. - record.set(true); CountDownLatch clientLatch = new CountDownLatch(count); AtomicInteger requests = new AtomicInteger(); for (int i = 0; i < count; ++i)