Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#7461 Fix flakey ConnectionPoolTest.testQueuedRequestsDontOpenTooManyConnections: #7521

Merged
merged 2 commits into from
Feb 14, 2022
Merged

#7461 Fix flakey ConnectionPoolTest.testQueuedRequestsDontOpenTooManyConnections: #7521

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3e8d56
#7461 Use less strict assertion for expiring pool to account for pot…
lorban Feb 2, 2022
f6c285d
#7461 Adapt other tests to use less strict assertion for expiring pool
lorban Feb 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 30 additions & 16 deletions jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
import org.junit.jupiter.params.provider.MethodSource;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

Expand All @@ -75,11 +77,6 @@ public static Stream<ConnectionPoolFactory> pools()
return Stream.of(DUPLEX, MULTIPLEX, RANDOM, DUPLEX_MAX_DURATION, ROUND_ROBIN);
}

public static Stream<ConnectionPoolFactory> poolsNoMaxDuration()
{
return Stream.of(DUPLEX, MULTIPLEX, RANDOM, ROUND_ROBIN);
}

public static Stream<ConnectionPoolFactory> poolsNoRoundRobin()
{
return Stream.of(DUPLEX, MULTIPLEX, RANDOM, DUPLEX_MAX_DURATION);
Expand Down Expand Up @@ -316,7 +313,10 @@ public void resolve(String host, int port, Promise<List<InetSocketAddress>> prom
assertEquals(1, destinations.size());
HttpDestination destination = (HttpDestination)destinations.get(0);
AbstractConnectionPool connectionPool = (AbstractConnectionPool)destination.getConnectionPool();
assertEquals(2, connectionPool.getConnectionCount());
if (DUPLEX_MAX_DURATION == factory)
assertThat(connectionPool.getConnectionCount(), lessThanOrEqualTo(2)); // The connections can expire upon release.
else
assertThat(connectionPool.getConnectionCount(), is(2));
lorban marked this conversation as resolved.
Show resolved Hide resolved
}

@ParameterizedTest
Expand Down Expand Up @@ -376,17 +376,15 @@ public void resolve(String host, int port, Promise<List<InetSocketAddress>> prom
}

@ParameterizedTest
// Connection pool aggressively closes expired connections upon release, which interferes with this test's assertion.
@MethodSource("poolsNoMaxDuration")
@MethodSource("pools")
public void testConcurrentRequestsAllBlockedOnServerWithLargeConnectionPool(ConnectionPoolFactory factory) throws Exception
{
int count = 50;
testConcurrentRequestsAllBlockedOnServer(factory, count, 2 * count);
}

@ParameterizedTest
// Connection pool aggressively closes expired connections upon release, which interferes with this test's assertion.
@MethodSource("poolsNoMaxDuration")
@MethodSource("pools")
public void testConcurrentRequestsAllBlockedOnServerWithExactConnectionPool(ConnectionPoolFactory factory) throws Exception
{
int count = 50;
Expand Down Expand Up @@ -449,9 +447,13 @@ protected void service(String target, org.eclipse.jetty.server.Request jettyRequ
assertTrue(latch.await(5, TimeUnit.SECONDS), "server requests " + barrier.getNumberWaiting() + "<" + count + " - client: " + client.dump());
List<Destination> destinations = client.getDestinations();
assertEquals(1, destinations.size());
HttpDestination destination = (HttpDestination)destinations.get(0);
AbstractConnectionPool connectionPool = (AbstractConnectionPool)destination.getConnectionPool();
assertThat(connectionPool.getConnectionCount(), Matchers.greaterThanOrEqualTo(count));
// The max duration connection pool aggressively closes expired connections upon release, which interferes with this assertion.
if (DUPLEX_MAX_DURATION != factory)
{
HttpDestination destination = (HttpDestination)destinations.get(0);
AbstractConnectionPool connectionPool = (AbstractConnectionPool)destination.getConnectionPool();
assertThat(connectionPool.getConnectionCount(), Matchers.greaterThanOrEqualTo(count));
}
}

@Test
Expand Down Expand Up @@ -588,8 +590,17 @@ public void testConnectionMaxUsage(ConnectionPoolFactory factory) throws Excepti
AbstractConnectionPool connectionPool = (AbstractConnectionPool)destination.getConnectionPool();

assertEquals(0, connectionPool.getActiveConnectionCount());
assertEquals(1, connectionPool.getIdleConnectionCount());
assertEquals(1, connectionPool.getConnectionCount());
if (DUPLEX_MAX_DURATION == factory)
{
// The connections can expire upon release.
assertThat(connectionPool.getIdleConnectionCount(), lessThanOrEqualTo(1));
assertThat(connectionPool.getConnectionCount(), lessThanOrEqualTo(1));
}
else
{
assertThat(connectionPool.getIdleConnectionCount(), is(1));
assertThat(connectionPool.getConnectionCount(), is(1));
}

// Send second request, max usage count will be reached,
// the only connection must be closed.
Expand Down Expand Up @@ -625,7 +636,10 @@ public void testIdleTimeoutNoRequests(ConnectionPoolFactory factory) throws Exce
// Trigger the creation of a destination, that will create the connection pool.
HttpDestination destination = client.resolveDestination(new Origin("http", "localhost", connector.getLocalPort()));
AbstractConnectionPool connectionPool = (AbstractConnectionPool)destination.getConnectionPool();
assertEquals(1, connectionPool.getConnectionCount());
if (DUPLEX_MAX_DURATION == factory)
assertThat(connectionPool.getConnectionCount(), lessThanOrEqualTo(1)); // The connections can expire upon release.
else
assertThat(connectionPool.getConnectionCount(), is(1));

// Wait for the pre-created connections to idle timeout.
Thread.sleep(idleTimeout + idleTimeout / 2);
Expand Down