Skip to content

Commit

Permalink
Fixes #2095 - Remove FastCGI multiplexing.
Browse files Browse the repository at this point in the history
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
sbordet committed Jan 11, 2019
1 parent bedb8e2 commit 64edaff
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@

package org.eclipse.jetty.fcgi.client.http;

import java.io.IOException;
import java.util.Map;

import org.eclipse.jetty.client.AbstractConnectorHttpClientTransport;
import org.eclipse.jetty.client.DuplexConnectionPool;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpDestination;
import org.eclipse.jetty.client.MultiplexConnectionPool;
import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.api.Connection;
import org.eclipse.jetty.client.api.Request;
Expand All @@ -40,35 +38,25 @@
@ManagedObject("The FastCGI/1.0 client transport")
public class HttpClientTransportOverFCGI extends AbstractConnectorHttpClientTransport
{
private final boolean multiplexed;
private final String scriptRoot;

public HttpClientTransportOverFCGI(String scriptRoot)
{
this( Math.max( 1, ProcessorUtils.availableProcessors() / 2), false, scriptRoot);
this(Math.max(1, ProcessorUtils.availableProcessors() / 2), scriptRoot);
}

public HttpClientTransportOverFCGI(int selectors, boolean multiplexed, String scriptRoot)
public HttpClientTransportOverFCGI(int selectors, String scriptRoot)
{
super(selectors);
this.multiplexed = multiplexed;
this.scriptRoot = scriptRoot;
setConnectionPoolFactory(destination ->
{
HttpClient httpClient = getHttpClient();
int maxConnections = httpClient.getMaxConnectionsPerDestination();
return isMultiplexed() ?
new MultiplexConnectionPool(destination, maxConnections, destination, httpClient.getMaxRequestsQueuedPerDestination()) :
new DuplexConnectionPool(destination, maxConnections, destination);
return new DuplexConnectionPool(destination, maxConnections, destination);
});
}

@ManagedAttribute(value = "Whether connections are multiplexed", readonly = true)
public boolean isMultiplexed()
{
return multiplexed;
}

@ManagedAttribute(value = "The scripts root directory", readonly = true)
public String getScriptRoot()
{
Expand All @@ -78,12 +66,11 @@ public String getScriptRoot()
@Override
public HttpDestination newHttpDestination(Origin origin)
{
return isMultiplexed() ? new MultiplexHttpDestinationOverFCGI(getHttpClient(), origin)
: new HttpDestinationOverFCGI(getHttpClient(), origin);
return new HttpDestinationOverFCGI(getHttpClient(), origin);
}

@Override
public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map<String, Object> context) throws IOException
public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map<String, Object> context)
{
HttpDestination destination = (HttpDestination)context.get(HTTP_DESTINATION_CONTEXT_KEY);
@SuppressWarnings("unchecked")
Expand All @@ -96,7 +83,7 @@ public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map<Stri

protected HttpConnectionOverFCGI newHttpConnection(EndPoint endPoint, HttpDestination destination, Promise<Connection> promise)
{
return new HttpConnectionOverFCGI(endPoint, destination, promise, isMultiplexed());
return new HttpConnectionOverFCGI(endPoint, destination, promise);
}

protected void customize(Request request, HttpFields fastCGIHeaders)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,16 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements Connec
private final AtomicBoolean closed = new AtomicBoolean();
private final HttpDestination destination;
private final Promise<Connection> promise;
private final boolean multiplexed;
private final Flusher flusher;
private final Delegate delegate;
private final ClientParser parser;
private ByteBuffer buffer;

public HttpConnectionOverFCGI(EndPoint endPoint, HttpDestination destination, Promise<Connection> promise, boolean multiplexed)
public HttpConnectionOverFCGI(EndPoint endPoint, HttpDestination destination, Promise<Connection> promise)
{
super(endPoint, destination.getHttpClient().getExecutor());
this.destination = destination;
this.promise = promise;
this.multiplexed = multiplexed;
this.flusher = new Flusher(endPoint);
this.delegate = new Delegate(destination);
this.parser = new ClientParser(new ResponseListener());
Expand Down Expand Up @@ -201,8 +199,6 @@ public boolean onIdleExpired()
{
long idleTimeout = getEndPoint().getIdleTimeout();
boolean close = delegate.onIdleTimeout(idleTimeout);
if (multiplexed)
close &= isFillInterested();
if (close)
close(new TimeoutException("Idle timeout " + idleTimeout + " ms"));
return false;
Expand Down Expand Up @@ -257,8 +253,6 @@ public boolean isClosed()

protected boolean closeByHTTP(HttpFields fields)
{
if (multiplexed)
return false;
if (!fields.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString()))
return false;
close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ private class ProxyHttpClientTransportOverFCGI extends HttpClientTransportOverFC
{
private ProxyHttpClientTransportOverFCGI(int selectors, String scriptRoot)
{
super(selectors, false, scriptRoot);
super(selectors, scriptRoot);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

package org.eclipse.jetty.fcgi.server;

import static org.hamcrest.MatcherAssert.assertThat;

import java.util.concurrent.atomic.AtomicLong;

import org.eclipse.jetty.client.HttpClient;
Expand All @@ -40,6 +38,8 @@
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;

import static org.hamcrest.MatcherAssert.assertThat;

public abstract class AbstractHttpClientServerTest
{
private LeakTrackingByteBufferPool serverBufferPool;
Expand Down Expand Up @@ -67,7 +67,7 @@ public void start(Handler handler) throws Exception
QueuedThreadPool executor = new QueuedThreadPool();
executor.setName(executor.getName() + "-client");

HttpClientTransport transport = new HttpClientTransportOverFCGI(1, false, "");
HttpClientTransport transport = new HttpClientTransportOverFCGI(1, "");
transport.setConnectionPoolFactory(destination -> new LeakTrackingConnectionPool(destination, client.getMaxConnectionsPerDestination(), destination)
{
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

package org.eclipse.jetty.http.client;

import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -49,6 +47,8 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;

import static org.junit.jupiter.api.Assertions.assertTrue;

public class HttpChannelAssociationTest extends AbstractTest<TransportScenario>
{
@Override
Expand Down Expand Up @@ -173,12 +173,12 @@ public boolean associate(HttpExchange exchange)
}
case FCGI:
{
return new HttpClientTransportOverFCGI(1, false, "")
return new HttpClientTransportOverFCGI(1, "")
{
@Override
protected HttpConnectionOverFCGI newHttpConnection(EndPoint endPoint, HttpDestination destination, Promise<Connection> promise)
{
return new HttpConnectionOverFCGI(endPoint, destination, promise, isMultiplexed())
return new HttpConnectionOverFCGI(endPoint, destination, promise)
{
@Override
protected HttpChannelOverFCGI newHttpChannel(Request request)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@

package org.eclipse.jetty.http.client;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
import java.io.InterruptedIOException;
import java.nio.ByteBuffer;
Expand Down Expand Up @@ -68,6 +65,9 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class HttpClientLoadTest extends AbstractTest<HttpClientLoadTest.LoadTransportScenario>
{
private final Logger logger = Log.getLogger(HttpClientLoadTest.class);
Expand Down Expand Up @@ -402,7 +402,7 @@ protected void leaked(LeakDetector.LeakInfo leakInfo)
}
case FCGI:
{
HttpClientTransport clientTransport = new HttpClientTransportOverFCGI(1, false, "");
HttpClientTransport clientTransport = new HttpClientTransportOverFCGI(1, "");
clientTransport.setConnectionPoolFactory(destination -> new LeakTrackingConnectionPool(destination, client.getMaxConnectionsPerDestination(), destination)
{
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public HttpClientTransport provideClientTransport(Transport transport)
}
case FCGI:
{
return new HttpClientTransportOverFCGI(1, false, "");
return new HttpClientTransportOverFCGI(1, "");
}
case UNIX_SOCKET:
{
Expand Down

0 comments on commit 64edaff

Please sign in to comment.