Skip to content

Commit

Permalink
Issue #6603 - HTTP/2 max local stream count exceeded
Browse files Browse the repository at this point in the history 
Incorporated changes from #6648.

Now the maxMultiple value is pulled from its primary value,
i.e. HTTP2Session.maxLocalStreams, rather than being set
in multiple places.

Deprecated usages of maxMultiplex and maxUsageCount.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
@sbordet
sbordet committed Aug 27, 2021
2 parents e27f192 + c3a19a6 commit 7733d0f
Show file tree
Hide file tree
Showing 12 changed files with 394 additions and 234 deletions.
20 changes: 7 additions & 13 deletions jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java
View file
Expand Up @@ -70,14 +70,20 @@ protected AbstractConnectionPool(Destination destination, int maxConnections, Ca

protected AbstractConnectionPool(HttpDestination destination, int maxConnections, boolean cache, Callback requester)
{
this(destination, new Pool<>(Pool.StrategyType.FIRST, maxConnections, cache), requester);
this(destination, Pool.StrategyType.FIRST, maxConnections, cache, requester);
}

protected AbstractConnectionPool(HttpDestination destination, Pool.StrategyType strategy, int maxConnections, boolean cache, Callback requester)
{
this(destination, new Pool<>(strategy, maxConnections, cache), requester);
}

protected AbstractConnectionPool(HttpDestination destination, Pool<Connection> pool, Callback requester)
{
this.destination = destination;
this.requester = requester;
this.pool = pool;
pool.setMaxMultiplex(1); // Force the use of multiplexing.
addBean(pool);
}

Expand Down Expand Up @@ -140,16 +146,6 @@ protected void setMaxMultiplex(int maxMultiplex)
pool.setMaxMultiplex(maxMultiplex);
}

protected void setMaxMultiplex(Connection connection, int maxMultiplex)
{
if (connection instanceof Attachable)
{
Object attachment = ((Attachable)connection).getAttachment();
if (attachment instanceof EntryHolder)
((EntryHolder)attachment).entry.setMaxMultiplex(maxMultiplex);
}
}

protected int getMaxUsageCount()
{
return pool.getMaxUsageCount();
Expand Down Expand Up @@ -538,8 +534,6 @@ public void succeeded(Connection connection)
onCreated(connection);
pending.decrementAndGet();
reserved.enable(connection, false);
if (connection instanceof Multiplexable)
setMaxMultiplex(connection, ((ConnectionPool.Multiplexable)connection).getMaxMultiplex());
idle(connection, false);
complete(null);
proceed();
Expand Down
12 changes: 8 additions & 4 deletions jetty-client/src/main/java/org/eclipse/jetty/client/ConnectionPool.java
View file
Expand Up @@ -29,7 +29,7 @@
public interface ConnectionPool extends Closeable
{
/**
* Optionally pre-create up to <code>connectionCount</code>
* Optionally pre-create up to {@code connectionCount}
* connections so they are immediately ready for use.
* @param connectionCount the number of connections to pre-start.
*/
Expand Down Expand Up @@ -109,13 +109,17 @@ interface Factory
interface Multiplexable
{
/**
* @return the max number of requests that can be multiplexed on a connection
* @return the max number of requests multiplexable on a single connection
*/
int getMaxMultiplex();

/**
* @param maxMultiplex the max number of requests that can be multiplexed on a connection
* @param maxMultiplex the max number of requests multiplexable on a single connection
* @deprecated do not use, as the maxMultiplex value is pulled, rather than pushed
*/
void setMaxMultiplex(int maxMultiplex);
@Deprecated
default void setMaxMultiplex(int maxMultiplex)
{
}
}
}
3 changes: 2 additions & 1 deletion jetty-client/src/main/java/org/eclipse/jetty/client/DuplexConnectionPool.java
View file
Expand Up @@ -34,9 +34,10 @@ public DuplexConnectionPool(HttpDestination destination, int maxConnections, Cal

public DuplexConnectionPool(HttpDestination destination, int maxConnections, boolean cache, Callback requester)
{
this(destination, new Pool<>(Pool.StrategyType.FIRST, maxConnections, cache), requester);
super(destination, Pool.StrategyType.FIRST, maxConnections, cache, requester);
}

@Deprecated
public DuplexConnectionPool(HttpDestination destination, Pool<Connection> pool, Callback requester)
{
super(destination, pool, requester);
Expand Down
19 changes: 18 additions & 1 deletion jetty-client/src/main/java/org/eclipse/jetty/client/MultiplexConnectionPool.java
View file
Expand Up @@ -34,9 +34,26 @@ public MultiplexConnectionPool(HttpDestination destination, int maxConnections,

public MultiplexConnectionPool(HttpDestination destination, int maxConnections, boolean cache, Callback requester, int maxMultiplex)
{
this(destination, new Pool<>(Pool.StrategyType.FIRST, maxConnections, cache), requester, maxMultiplex);
this(destination, Pool.StrategyType.FIRST, maxConnections, cache, requester, maxMultiplex);
}

public MultiplexConnectionPool(HttpDestination destination, Pool.StrategyType strategy, int maxConnections, boolean cache, Callback requester, int maxMultiplex)
{
super(destination, new Pool<Connection>(strategy, maxConnections, cache)
{
@Override
protected int getMaxMultiplex(Connection connection)
{
int multiplex = (connection instanceof Multiplexable)
? ((Multiplexable)connection).getMaxMultiplex()
: super.getMaxMultiplex(connection);
return multiplex > 0 ? multiplex : 1;
}
}, requester);
setMaxMultiplex(maxMultiplex);
}

@Deprecated
public MultiplexConnectionPool(HttpDestination destination, Pool<Connection> pool, Callback requester, int maxMultiplex)
{
super(destination, pool, requester);
Expand Down
9 changes: 0 additions & 9 deletions jetty-client/src/main/java/org/eclipse/jetty/client/MultiplexHttpDestination.java
View file
Expand Up @@ -18,8 +18,6 @@

package org.eclipse.jetty.client;

import org.eclipse.jetty.client.api.Connection;

public abstract class MultiplexHttpDestination extends HttpDestination
{
protected MultiplexHttpDestination(HttpClient client, Origin origin)
Expand All @@ -41,11 +39,4 @@ public void setMaxRequestsPerConnection(int maxRequestsPerConnection)
if (connectionPool instanceof AbstractConnectionPool)
((AbstractConnectionPool)connectionPool).setMaxMultiplex(maxRequestsPerConnection);
}

public void setMaxRequestsPerConnection(Connection connection, int maxRequestsPerConnection)
{
ConnectionPool connectionPool = getConnectionPool();
if (connectionPool instanceof AbstractConnectionPool)
((AbstractConnectionPool)connectionPool).setMaxMultiplex(connection, maxRequestsPerConnection);
}
}
2 changes: 1 addition & 1 deletion jetty-client/src/main/java/org/eclipse/jetty/client/RandomConnectionPool.java
View file
Expand Up @@ -31,6 +31,6 @@ public class RandomConnectionPool extends MultiplexConnectionPool
{
public RandomConnectionPool(HttpDestination destination, int maxConnections, Callback requester, int maxMultiplex)
{
super(destination, new Pool<>(Pool.StrategyType.RANDOM, maxConnections, false), requester, maxMultiplex);
super(destination, Pool.StrategyType.RANDOM, maxConnections, false, requester, maxMultiplex);
}
}
2 changes: 1 addition & 1 deletion jetty-client/src/main/java/org/eclipse/jetty/client/RoundRobinConnectionPool.java
View file
Expand Up @@ -56,7 +56,7 @@ public RoundRobinConnectionPool(HttpDestination destination, int maxConnections,

public RoundRobinConnectionPool(HttpDestination destination, int maxConnections, Callback requester, int maxMultiplex)
{
super(destination, new Pool<>(Pool.StrategyType.ROUND_ROBIN, maxConnections, false), requester, maxMultiplex);
super(destination, Pool.StrategyType.ROUND_ROBIN, maxConnections, false, requester, maxMultiplex);
// If there are queued requests and connections get
// closed due to idle timeout or overuse, we want to
// aggressively try to open new connections to replace
Expand Down
19 changes: 3 additions & 16 deletions ...sport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java
View file
Expand Up @@ -211,26 +211,13 @@ private Promise<Connection> connectionPromise()
@Override
public void onSettings(Session session, SettingsFrame frame)
{
Map<Integer, Integer> settings = frame.getSettings();
Integer maxConcurrentStreams = settings.get(SettingsFrame.MAX_CONCURRENT_STREAMS);
boolean[] initialized = new boolean[1];
Connection connection = this.connection.get(initialized);
if (initialized[0])
{
if (maxConcurrentStreams != null && connection != null)
destination().setMaxRequestsPerConnection(connection, maxConcurrentStreams);
}
else
{
onServerPreface(session, maxConcurrentStreams);
}
if (!connection.isMarked())
onServerPreface(session);
}

private void onServerPreface(Session session, Integer maxConcurrentStreams)
private void onServerPreface(Session session)
{
HttpConnectionOverHTTP2 connection = newHttpConnection(destination(), session);
if (maxConcurrentStreams != null)
connection.setMaxMultiplex(maxConcurrentStreams);
if (this.connection.compareAndSet(null, connection, false, true))
connectionPromise().succeeded(connection);
}
Expand Down
10 changes: 3 additions & 7 deletions ...-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java
View file
Expand Up @@ -37,6 +37,7 @@
import org.eclipse.jetty.client.SendFailure;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http2.ErrorCode;
import org.eclipse.jetty.http2.HTTP2Session;
import org.eclipse.jetty.http2.api.Session;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.log.Log;
Expand All @@ -53,7 +54,6 @@ public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.S
private final AtomicInteger sweeps = new AtomicInteger();
private final Session session;
private boolean recycleHttpChannels = true;
private int maxMultiplex = 1;

public HttpConnectionOverHTTP2(HttpDestination destination, Session session)
{
Expand All @@ -76,14 +76,10 @@ public void setRecycleHttpChannels(boolean recycleHttpChannels)
this.recycleHttpChannels = recycleHttpChannels;
}

@Override
public int getMaxMultiplex()
{
return maxMultiplex;
}

public void setMaxMultiplex(int maxMultiplex)
{
this.maxMultiplex = maxMultiplex;
return ((HTTP2Session)session).getMaxLocalStreams();
}

@Override
Expand Down
10 changes: 4 additions & 6 deletions ...port/src/test/java/org/eclipse/jetty/http2/client/http/MultiplexedConnectionPoolTest.java
View file
Expand Up @@ -100,9 +100,7 @@ public void testMaxDurationConnectionsWithMultiplexedPool() throws Exception
ConnectionPoolFactory factory = new ConnectionPoolFactory("duplex-maxDuration", destination ->
{
int maxConnections = destination.getHttpClient().getMaxConnectionsPerDestination();
Pool<Connection> pool = new Pool<>(Pool.StrategyType.FIRST, maxConnections, false);
poolRef.set(pool);
MultiplexConnectionPool connectionPool = new MultiplexConnectionPool(destination, pool, destination, MAX_MULTIPLEX)
MultiplexConnectionPool connectionPool = new MultiplexConnectionPool(destination, Pool.StrategyType.FIRST, maxConnections, false, destination, MAX_MULTIPLEX)
{
@Override
protected void onCreated(Connection connection)
Expand All @@ -116,6 +114,7 @@ protected void removed(Connection connection)
poolRemoveCounter.incrementAndGet();
}
};
poolRef.set(connectionPool.getBean(Pool.class));
connectionPool.setMaxDuration(maxDuration);
return connectionPool;
});
Expand Down Expand Up @@ -161,9 +160,7 @@ public void testMaxDurationConnectionsWithMultiplexedPoolClosesExpiredConnection
ConnectionPoolFactory factory = new ConnectionPoolFactory("duplex-maxDuration", destination ->
{
int maxConnections = destination.getHttpClient().getMaxConnectionsPerDestination();
Pool<Connection> pool = new Pool<>(Pool.StrategyType.FIRST, maxConnections, false);
poolRef.set(pool);
MultiplexConnectionPool connectionPool = new MultiplexConnectionPool(destination, pool, destination, MAX_MULTIPLEX)
MultiplexConnectionPool connectionPool = new MultiplexConnectionPool(destination, Pool.StrategyType.FIRST, maxConnections, false, destination, MAX_MULTIPLEX)
{
@Override
protected void onCreated(Connection connection)
Expand All @@ -177,6 +174,7 @@ protected void removed(Connection connection)
poolRemoveCounter.incrementAndGet();
}
};
poolRef.set(connectionPool.getBean(Pool.class));
connectionPool.setMaxDuration(maxDuration);
return connectionPool;
});
Expand Down

0 comments on commit 7733d0f

Please sign in to comment.