From 001c1d6da54d2b052b5b677356be43136c4d8474 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 12 Dec 2018 12:03:25 +0100 Subject: Revert "Pass correct argument to addBean" This reverts commit b821372e19a98e9d87d6020e2208585863fb22ed. --- .../java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java index 58d1142c563..79a4f93d1df 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java @@ -46,7 +46,7 @@ class ConnectionThrottler { } void registerBeans() { - beans.forEach(bean -> connector.getServer().addBean(bean)); + beans.forEach(bean -> connector.getServer().addBean(connector)); } private static Duration fromSeconds(double seconds) { -- cgit v1.2.3 From 7cb63ebe0c9eabc23a569044ec6d9fbf3f1a5a13 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 12 Dec 2018 12:04:21 +0100 Subject: Revert "Change memory threshold to a percentage of total memory" This reverts commit 2db810113ffc50b26cae63c034cdf0f33b859c64. --- .../yahoo/jdisc/http/server/jetty/ConnectionThrottler.java | 14 +++++--------- .../resources/configdefinitions/jdisc.http.connector.def | 4 ++-- .../com/yahoo/jdisc/http/server/jetty/HttpServerTest.java | 2 +- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java index 79a4f93d1df..370ac0aa788 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java @@ -40,8 +40,8 @@ class ConnectionThrottler { if (config.maxConnections() != -1) { beans.add(new CoordinatedConnectionLimit(config.maxConnections(), idleTimeout)); } - if (config.maxHeapUtilization() != -1) { - beans.add(new CoordinatedLowResourcesLimit(config.maxHeapUtilization(), idleTimeout)); + if (config.maxMemoryUsage() != -1) { + beans.add(new CoordinatedLowResourcesLimit(config.maxMemoryUsage(), idleTimeout)); } } @@ -70,16 +70,12 @@ class ConnectionThrottler { } resetters.forEach(Runnable::run); } - private static long toMaxMemoryUsageInBytes(double maxHeapUtilization) { - return (long) (maxHeapUtilization * Runtime.getRuntime().maxMemory()); - } private class CoordinatedLowResourcesLimit extends LowResourceMonitor { - - CoordinatedLowResourcesLimit(double maxHeapUtilization, Duration idleTimeout) { + CoordinatedLowResourcesLimit(int maxMemoryUsageMegaBytes, Duration idleTimeout) { super(connector.getServer()); super.setMonitoredConnectors(singleton(connector)); - super.setMaxMemory(toMaxMemoryUsageInBytes(maxHeapUtilization)); + super.setMaxMemory(maxMemoryUsageMegaBytes * 1024 * 1024L); super.setLowResourcesIdleTimeout((int)idleTimeout.toMillis()); } @@ -94,8 +90,8 @@ class ConnectionThrottler { ConnectionThrottler.this.onReset(); } } - private class CoordinatedConnectionLimit extends ConnectionLimit { + private class CoordinatedConnectionLimit extends ConnectionLimit { CoordinatedConnectionLimit(int maxConnections, Duration idleTimeout) { super(maxConnections, connector); super.setIdleTimeout(idleTimeout.toMillis()); diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def index 7967f657aff..ae4e66a236b 100644 --- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def +++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def @@ -47,8 +47,8 @@ throttling.enabled bool default=false # Max number of connections. throttling.maxConnections int default=-1 -# Max memory utilization as a value between 0 and 1. -throttling.maxHeapUtilization double default=-1 +# Max memory usage in megabytes (totalMemory - freeMemory). +throttling.maxMemoryUsage int default=-1 # Max connection accept rate. throttling.maxAcceptRate int default=-1 diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index 9622edc5429..479cf514e30 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -490,7 +490,7 @@ public class HttpServerTest { .throttling(new Throttling.Builder() .enabled(true) .maxAcceptRate(10) - .maxHeapUtilization(0.99) + .maxMemoryUsage(100*1024) .maxConnections(10))); driver.client().get("/status.html") .expectStatusCode(is(OK)); -- cgit v1.2.3 From d069b916a89a2e40800a6a143aeef4e73826473c Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 12 Dec 2018 12:04:41 +0100 Subject: Revert "Add support for connection throttling in JDisc" This reverts commit 36221bb67238256d46cb0fe69ca682172d2bec65. --- .../http/server/jetty/ConnectionThrottler.java | 128 --------------------- .../http/server/jetty/JDiscServerConnector.java | 4 - .../configdefinitions/jdisc.http.connector.def | 18 --- .../jdisc/http/server/jetty/HttpServerTest.java | 17 --- 4 files changed, 167 deletions(-) delete mode 100644 jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java deleted file mode 100644 index 370ac0aa788..00000000000 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionThrottler.java +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.jdisc.http.server.jetty; - -import com.yahoo.jdisc.http.ConnectorConfig; -import org.eclipse.jetty.server.AcceptRateLimit; -import org.eclipse.jetty.server.ConnectionLimit; -import org.eclipse.jetty.server.Connector; -import org.eclipse.jetty.server.LowResourceMonitor; -import org.eclipse.jetty.util.component.LifeCycle; - -import java.time.Duration; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Queue; -import java.util.concurrent.TimeUnit; - -import static java.util.Collections.singleton; - -/** - * Throttles new connections using {@link LowResourceMonitor}, {@link AcceptRateLimit} and {@link ConnectionLimit}. - * - * @author bjorncs - */ -class ConnectionThrottler { - - private final Object monitor = new Object(); - private final Queue throttleResetters = new ArrayDeque<>(); - private final Collection beans = new ArrayList<>(); - private final Connector connector; - private int throttlersCount; - - ConnectionThrottler(Connector connector, ConnectorConfig.Throttling config) { - this.connector = connector; - Duration idleTimeout = fromSeconds(config.idleTimeout()); - if (config.maxAcceptRate() != -1) { - beans.add(new CoordinatedAcceptRateLimit(config.maxAcceptRate(), fromSeconds(config.maxAcceptRatePeriod()))); - } - if (config.maxConnections() != -1) { - beans.add(new CoordinatedConnectionLimit(config.maxConnections(), idleTimeout)); - } - if (config.maxMemoryUsage() != -1) { - beans.add(new CoordinatedLowResourcesLimit(config.maxMemoryUsage(), idleTimeout)); - } - } - - void registerBeans() { - beans.forEach(bean -> connector.getServer().addBean(connector)); - } - - private static Duration fromSeconds(double seconds) { - return Duration.ofMillis((long) (seconds * 1000)); - } - - private void onThrottle(Runnable throttleResetter) { - synchronized (monitor) { - ++throttlersCount; - throttleResetters.offer(throttleResetter); - } - } - - private void onReset() { - List resetters = new ArrayList<>(); - synchronized (monitor) { - if (--throttlersCount == 0) { - resetters.addAll(throttleResetters); - throttleResetters.clear(); - } - } - resetters.forEach(Runnable::run); - } - - private class CoordinatedLowResourcesLimit extends LowResourceMonitor { - CoordinatedLowResourcesLimit(int maxMemoryUsageMegaBytes, Duration idleTimeout) { - super(connector.getServer()); - super.setMonitoredConnectors(singleton(connector)); - super.setMaxMemory(maxMemoryUsageMegaBytes * 1024 * 1024L); - super.setLowResourcesIdleTimeout((int)idleTimeout.toMillis()); - } - - @Override - protected void setLowResources() { - super.setLowResources(); - ConnectionThrottler.this.onThrottle(() -> super.clearLowResources()); - } - - @Override - protected void clearLowResources() { - ConnectionThrottler.this.onReset(); - } - } - - private class CoordinatedConnectionLimit extends ConnectionLimit { - CoordinatedConnectionLimit(int maxConnections, Duration idleTimeout) { - super(maxConnections, connector); - super.setIdleTimeout(idleTimeout.toMillis()); - } - - @Override - protected void limit() { - super.limit(); - ConnectionThrottler.this.onThrottle(() -> super.unlimit()); - } - - @Override - protected void unlimit() { - ConnectionThrottler.this.onReset(); - } - } - - private class CoordinatedAcceptRateLimit extends AcceptRateLimit { - CoordinatedAcceptRateLimit(int limit, Duration period) { - super(limit, period.toMillis(), TimeUnit.MILLISECONDS, connector); - } - - @Override - protected void limit() { - super.limit(); - ConnectionThrottler.this.onThrottle(() -> super.unlimit()); - } - - @Override - protected void unlimit() { - ConnectionThrottler.this.onReset(); - } - } -} diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java index a80e694ed30..f7d6e1717af 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java @@ -50,10 +50,6 @@ class JDiscServerConnector extends ServerConnector { this.statistics = new ServerConnectionStatistics(); addBean(statistics); - ConnectorConfig.Throttling throttlingConfig = config.throttling(); - if (throttlingConfig.enabled()) { - new ConnectionThrottler(this, throttlingConfig).registerBeans(); - } } @Override diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def index ae4e66a236b..157ffabdd63 100644 --- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def +++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def @@ -41,24 +41,6 @@ tcpKeepAliveEnabled bool default=false # Enable/disable TCP_NODELAY (disable/enable Nagle's algorithm). tcpNoDelay bool default=true -# Whether to enable connection throttling. New connections will be dropped when a threshold is exceeded. -throttling.enabled bool default=false - -# Max number of connections. -throttling.maxConnections int default=-1 - -# Max memory usage in megabytes (totalMemory - freeMemory). -throttling.maxMemoryUsage int default=-1 - -# Max connection accept rate. -throttling.maxAcceptRate int default=-1 - -# Accept rate sample period in seconds. Used in conjunction with throttling.maxAcceptRate. -throttling.maxAcceptRatePeriod double default=1.0 - -# Idle timeout in seconds applied to endpoints when a threshold is exceeded (except accept rate threshold). -throttling.idleTimeout double default=1.0 - # Whether to enable SSL for this connector. ssl.enabled bool default=false diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index 479cf514e30..30d5f9e657a 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -16,7 +16,6 @@ import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.handler.ResponseDispatch; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.ConnectorConfig; -import com.yahoo.jdisc.http.ConnectorConfig.Throttling; import com.yahoo.jdisc.http.Cookie; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.HttpResponse; @@ -481,22 +480,6 @@ public class HttpServerTest { assertThat(driver.close(), is(true)); } - @Test - public void requireThatConnectionThrottleDoesNotBlockConnectionsBelowThreshold() throws Exception { - final TestDriver driver = TestDrivers.newConfiguredInstance( - new EchoRequestHandler(), - new ServerConfig.Builder(), - new ConnectorConfig.Builder() - .throttling(new Throttling.Builder() - .enabled(true) - .maxAcceptRate(10) - .maxMemoryUsage(100*1024) - .maxConnections(10))); - driver.client().get("/status.html") - .expectStatusCode(is(OK)); - assertThat(driver.close(), is(true)); - } - private static RequestHandler mockRequestHandler() { final RequestHandler mockRequestHandler = mock(RequestHandler.class); when(mockRequestHandler.refer()).thenReturn(References.NOOP_REFERENCE); -- cgit v1.2.3