Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

433689 - Evict old HttpDestinations from HttpClient.

This commit is contained in:
Simone Bordet 2014-05-14 11:34:34 +02:00
parent a803465551
commit be3848a371
5 changed files with 91 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
jetty-client/src/main/java/org/eclipse/jetty/client/ConnectionPool.java
View File

@ -192,6 +192,11 @@ public class ConnectionPool implements Closeable, Dumpable
return idleConnections.contains(connection);
}
public boolean isEmpty()
{
return connectionCount.get() == 0;
}
public void close()
{
for (Connection connection : idleConnections)

32
jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java
View File

@ -133,6 +133,7 @@ public class HttpClient extends ContainerLifeCycle
private volatile boolean dispatchIO = true;
private volatile boolean strictEventOrdering = false;
private volatile HttpField encodingField;
private volatile boolean removeIdleDestinations = false;
/**
* Creates a {@link HttpClient} instance that can perform requests to non-TLS destinations only
@ -464,6 +465,11 @@ public class HttpClient extends ContainerLifeCycle
return destination;
}
protected boolean removeDestination(HttpDestination destination)
{
return destinations.remove(destination.getOrigin()) != null;
}
/**
* @return the list of destinations known to this {@link HttpClient}.
*/
@ -863,6 +869,32 @@ public class HttpClient extends ContainerLifeCycle
this.strictEventOrdering = strictEventOrdering;
}
/**
* @return whether destinations that have no connections should be removed
* @see #setRemoveIdleDestinations(boolean)
*/
public boolean isRemoveIdleDestinations()
{
return removeIdleDestinations;
}
/**
* Whether destinations that have no connections (nor active nor idle) should be removed.
* <p />
* Applications typically make request to a limited number of destinations so keeping
* destinations around is not a problem for the memory or the GC.
* However, for applications that hit millions of different destinations (e.g. a spider
* bot) it would be useful to be able to remove the old destinations that won't be visited
* anymore and leave space for new destinations.
*
* @param removeIdleDestinations whether destinations that have no connections should be removed
* @see org.eclipse.jetty.client.ConnectionPool
*/
public void setRemoveIdleDestinations(boolean removeIdleDestinations)
{
this.removeIdleDestinations = removeIdleDestinations;
}
/**
* @return the forward proxy configuration
*/

4
jetty-client/src/main/java/org/eclipse/jetty/client/MultiplexHttpDestination.java
View File

@ -146,7 +146,11 @@ public abstract class MultiplexHttpDestination<C extends Connection> extends Htt
{
ConnectState current = connect.get();
if (connect.compareAndSet(current, ConnectState.DISCONNECTED))
{
if (getHttpClient().isRemoveIdleDestinations())
getHttpClient().removeDestination(this);
break;
}
}
}

21
jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java
View File

@ -168,11 +168,24 @@ public abstract class PoolingHttpDestination<C extends Connection> extends HttpD
super.close(oldConnection);
connectionPool.remove(oldConnection);
// We need to execute queued requests even if this connection failed.
// We may create a connection that is not needed, but it will eventually
// idle timeout, so no worries
if (!getHttpExchanges().isEmpty())
if (getHttpExchanges().isEmpty())
{
if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty())
{
// There is a race condition between this thread removing the destination
// and another thread queueing a request to this same destination.
// If this destination is removed, but the request queued, a new connection
// will be opened, the exchange will be executed and eventually the connection
// will idle timeout and be closed. Meanwhile a new destination will be created
// in HttpClient and will be used for other requests.
getHttpClient().removeDestination(this);
}
}
else
{
// We need to execute queued requests even if this connection failed.
// We may create a connection that is not needed, but it will eventually
// idle timeout, so no worries.
C newConnection = acquire();
if (newConnection != null)
process(newConnection, false);

33
jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java
View File

@ -27,9 +27,12 @@ import org.eclipse.jetty.client.EmptyServerHandler;
import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.api.Connection;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Destination;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.toolchain.test.annotation.Slow;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.hamcrest.Matchers;
@ -226,4 +229,34 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
Assert.assertTrue(failureLatch.await(5, TimeUnit.SECONDS));
Assert.assertTrue(successLatch.await(5, TimeUnit.SECONDS));
}
@Test
public void testDestinationIsRemoved() throws Exception
{
String host = "localhost";
int port = connector.getLocalPort();
Destination destinationBefore = client.getDestination(scheme, host, port);
ContentResponse response = client.newRequest(host, port)
.scheme(scheme)
.header(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString())
.send();
Assert.assertEquals(200, response.getStatus());
Destination destinationAfter = client.getDestination(scheme, host, port);
Assert.assertSame(destinationBefore, destinationAfter);
client.setRemoveIdleDestinations(true);
response = client.newRequest(host, port)
.scheme(scheme)
.header(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString())
.send();
Assert.assertEquals(200, response.getStatus());
destinationAfter = client.getDestination(scheme, host, port);
Assert.assertNotSame(destinationBefore, destinationAfter);
}
}