This is an automated email from the ASF dual-hosted git repository. tingchen pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push: new b7e93ac14b Add Support for Tuning HTTP Server Thread Pool (#10001) b7e93ac14b is described below commit b7e93ac14b211678a3d602c73dcf746604a55fe8 Author: Ankit Sultana <ankitsult...@uber.com> AuthorDate: Fri Dec 30 21:26:09 2022 +0530 Add Support for Tuning HTTP Server Thread Pool (#10001) * Add Support for Tuning HTTP Server Thread Pool * Fix UT * Rename to HttpServerThreadPoolConfig * Fix lint * Fix lint again * rename config to http.server.thread.pool --- .../controller/util/ListenerConfigUtilTest.java | 52 +++++++++++++++---- .../core/transport/HttpServerThreadPoolConfig.java | 59 ++++++++++++++++++++++ .../pinot/core/transport/ListenerConfig.java | 9 +++- .../apache/pinot/core/util/ListenerConfigUtil.java | 39 ++++++++++---- .../apache/pinot/server/api/AccessControlTest.java | 3 +- .../apache/pinot/server/api/BaseResourceTest.java | 3 +- 6 files changed, 144 insertions(+), 21 deletions(-) diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/util/ListenerConfigUtilTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/util/ListenerConfigUtilTest.java index 9384604b75..dae4c5f4e7 100644 --- a/pinot-controller/src/test/java/org/apache/pinot/controller/util/ListenerConfigUtilTest.java +++ b/pinot-controller/src/test/java/org/apache/pinot/controller/util/ListenerConfigUtilTest.java @@ -21,6 +21,7 @@ package org.apache.pinot.controller.util; import com.google.common.collect.ImmutableList; import java.util.List; import org.apache.pinot.controller.ControllerConf; +import org.apache.pinot.core.transport.HttpServerThreadPoolConfig; import org.apache.pinot.core.transport.ListenerConfig; import org.apache.pinot.core.util.ListenerConfigUtil; import org.apache.pinot.spi.env.PinotConfiguration; @@ -51,6 +52,30 @@ public class ListenerConfigUtilTest { ListenerConfigUtil.buildControllerConfigs(new ControllerConf()); } + @Test + public void testThreadPoolConfig() { + ControllerConf controllerConf = new ControllerConf(); + + controllerConf.setProperty("controller.port", "9000"); + + // When server thread pool config is not set, default configs should be used + List<ListenerConfig> listenerConfigs = ListenerConfigUtil.buildControllerConfigs(controllerConf); + Assert.assertEquals(listenerConfigs.size(), 1); + Assert.assertEquals(HttpServerThreadPoolConfig.defaultInstance().getCorePoolSize(), + listenerConfigs.get(0).getThreadPoolConfig().getCorePoolSize()); + Assert.assertEquals(HttpServerThreadPoolConfig.defaultInstance().getMaxPoolSize(), + listenerConfigs.get(0).getThreadPoolConfig().getMaxPoolSize()); + + // Set server thread pool configs and assert that they are set + controllerConf.setProperty("controller.http.server.thread.pool.corePoolSize", 7); + controllerConf.setProperty("controller.http.server.thread.pool.maxPoolSize", 9); + + listenerConfigs = ListenerConfigUtil.buildControllerConfigs(controllerConf); + Assert.assertEquals(listenerConfigs.size(), 1); + Assert.assertEquals(7, listenerConfigs.get(0).getThreadPoolConfig().getCorePoolSize()); + Assert.assertEquals(9, listenerConfigs.get(0).getThreadPoolConfig().getMaxPoolSize()); + } + /** * Asserts that enabling https generates the existing legacy listener as well as the another one configured with * TLS settings. @@ -176,21 +201,30 @@ public class ListenerConfigUtilTest { @Test public void testFindLastTlsPort() { - List<ListenerConfig> configs = ImmutableList.of(new ListenerConfig("conf1", "host1", 9000, "http", null), - new ListenerConfig("conf2", "host2", 9001, "https", null), - new ListenerConfig("conf3", "host3", 9002, "http", null), - new ListenerConfig("conf4", "host4", 9003, "https", null), - new ListenerConfig("conf5", "host5", 9004, "http", null)); + List<ListenerConfig> configs = ImmutableList.of(new ListenerConfig("conf1", "host1", 9000, "http", null, + HttpServerThreadPoolConfig.defaultInstance()), + new ListenerConfig("conf2", "host2", 9001, "https", null, + HttpServerThreadPoolConfig.defaultInstance()), + new ListenerConfig("conf3", "host3", 9002, "http", null, + HttpServerThreadPoolConfig.defaultInstance()), + new ListenerConfig("conf4", "host4", 9003, "https", null, + HttpServerThreadPoolConfig.defaultInstance()), + new ListenerConfig("conf5", "host5", 9004, "http", null, + HttpServerThreadPoolConfig.defaultInstance())); int tlsPort = ListenerConfigUtil.findLastTlsPort(configs, -1); Assert.assertEquals(tlsPort, 9003); } @Test public void testFindLastTlsPortMissing() { - List<ListenerConfig> configs = ImmutableList.of(new ListenerConfig("conf1", "host1", 9000, "http", null), - new ListenerConfig("conf2", "host2", 9001, "http", null), - new ListenerConfig("conf3", "host3", 9002, "http", null), - new ListenerConfig("conf4", "host4", 9004, "http", null)); + List<ListenerConfig> configs = ImmutableList.of(new ListenerConfig("conf1", "host1", 9000, "http", null, + HttpServerThreadPoolConfig.defaultInstance()), + new ListenerConfig("conf2", "host2", 9001, "http", null, + HttpServerThreadPoolConfig.defaultInstance()), + new ListenerConfig("conf3", "host3", 9002, "http", null, + HttpServerThreadPoolConfig.defaultInstance()), + new ListenerConfig("conf4", "host4", 9004, "http", null, + HttpServerThreadPoolConfig.defaultInstance())); int tlsPort = ListenerConfigUtil.findLastTlsPort(configs, -1); Assert.assertEquals(tlsPort, -1); } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/transport/HttpServerThreadPoolConfig.java b/pinot-core/src/main/java/org/apache/pinot/core/transport/HttpServerThreadPoolConfig.java new file mode 100644 index 0000000000..c4bf6d5109 --- /dev/null +++ b/pinot-core/src/main/java/org/apache/pinot/core/transport/HttpServerThreadPoolConfig.java @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.core.transport; + +/** + * This configures the thread pool configs for the Http servers in Pinot server, controller, broker and minion. + */ +public class HttpServerThreadPoolConfig { + private static final HttpServerThreadPoolConfig DEFAULT = + new HttpServerThreadPoolConfig(Runtime.getRuntime().availableProcessors() * 2, + Runtime.getRuntime().availableProcessors() * 2); + private int _maxPoolSize; + private int _corePoolSize; + + public HttpServerThreadPoolConfig(int corePoolSize, int maxPoolSize) { + _maxPoolSize = maxPoolSize; + _corePoolSize = corePoolSize; + } + + public static HttpServerThreadPoolConfig defaultInstance() { + return DEFAULT.copy(); + } + + public int getMaxPoolSize() { + return _maxPoolSize; + } + + public void setMaxPoolSize(int maxPoolSize) { + _maxPoolSize = maxPoolSize; + } + + public int getCorePoolSize() { + return _corePoolSize; + } + + public void setCorePoolSize(int corePoolSize) { + _corePoolSize = corePoolSize; + } + + public HttpServerThreadPoolConfig copy() { + return new HttpServerThreadPoolConfig(_corePoolSize, _maxPoolSize); + } +} diff --git a/pinot-core/src/main/java/org/apache/pinot/core/transport/ListenerConfig.java b/pinot-core/src/main/java/org/apache/pinot/core/transport/ListenerConfig.java index 7c47d38b55..172efe02bb 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/transport/ListenerConfig.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/transport/ListenerConfig.java @@ -31,13 +31,16 @@ public class ListenerConfig { private final int _port; private final String _protocol; private final TlsConfig _tlsConfig; + private final HttpServerThreadPoolConfig _threadPoolConfig; - public ListenerConfig(String name, String host, int port, String protocol, TlsConfig tlsConfig) { + public ListenerConfig(String name, String host, int port, String protocol, TlsConfig tlsConfig, + HttpServerThreadPoolConfig threadPoolConfig) { _name = name; _host = host; _port = port; _protocol = protocol; _tlsConfig = tlsConfig; + _threadPoolConfig = threadPoolConfig; } public String getName() { @@ -59,4 +62,8 @@ public class ListenerConfig { public TlsConfig getTlsConfig() { return _tlsConfig; } + + public HttpServerThreadPoolConfig getThreadPoolConfig() { + return _threadPoolConfig; + } } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java b/pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java index b5e4eae166..1ce41e52cc 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java @@ -38,6 +38,7 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.apache.pinot.common.config.TlsConfig; import org.apache.pinot.common.utils.TlsUtils; +import org.apache.pinot.core.transport.HttpServerThreadPoolConfig; import org.apache.pinot.core.transport.ListenerConfig; import org.apache.pinot.spi.env.PinotConfiguration; import org.apache.pinot.spi.utils.CommonConstants; @@ -60,6 +61,7 @@ import static org.apache.pinot.spi.utils.CommonConstants.HTTPS_PROTOCOL; public final class ListenerConfigUtil { private static final String DEFAULT_HOST = "0.0.0.0"; private static final String DOT_ACCESS_PROTOCOLS = ".access.protocols"; + private static final String DOT_ACCESS_THREAD_POOL = ".http.server.thread.pool"; private ListenerConfigUtil() { // left blank @@ -96,7 +98,7 @@ public final class ListenerConfigUtil { String portString = controllerConf.getProperty("controller.port"); if (portString != null) { listeners.add(new ListenerConfig(CommonConstants.HTTP_PROTOCOL, DEFAULT_HOST, Integer.parseInt(portString), - CommonConstants.HTTP_PROTOCOL, new TlsConfig())); + CommonConstants.HTTP_PROTOCOL, new TlsConfig(), buildServerThreadPoolConfig(controllerConf, "controller"))); } TlsConfig tlsDefaults = TlsUtils.extractTlsConfig(controllerConf, "controller.tls"); @@ -113,7 +115,7 @@ public final class ListenerConfigUtil { String queryPortString = brokerConf.getProperty(CommonConstants.Helix.KEY_OF_BROKER_QUERY_PORT); if (queryPortString != null) { listeners.add(new ListenerConfig(CommonConstants.HTTP_PROTOCOL, DEFAULT_HOST, Integer.parseInt(queryPortString), - CommonConstants.HTTP_PROTOCOL, new TlsConfig())); + CommonConstants.HTTP_PROTOCOL, new TlsConfig(), buildServerThreadPoolConfig(brokerConf, "broker"))); } TlsConfig tlsDefaults = TlsUtils.extractTlsConfig(brokerConf, CommonConstants.Broker.BROKER_TLS_PREFIX); @@ -123,7 +125,8 @@ public final class ListenerConfigUtil { // support legacy behavior < 0.7.0 if (listeners.isEmpty()) { listeners.add(new ListenerConfig(CommonConstants.HTTP_PROTOCOL, DEFAULT_HOST, - CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT, CommonConstants.HTTP_PROTOCOL, new TlsConfig())); + CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT, CommonConstants.HTTP_PROTOCOL, new TlsConfig(), + buildServerThreadPoolConfig(brokerConf, "broker"))); } return listeners; @@ -136,7 +139,7 @@ public final class ListenerConfigUtil { if (adminApiPortString != null) { listeners.add( new ListenerConfig(CommonConstants.HTTP_PROTOCOL, DEFAULT_HOST, Integer.parseInt(adminApiPortString), - CommonConstants.HTTP_PROTOCOL, new TlsConfig())); + CommonConstants.HTTP_PROTOCOL, new TlsConfig(), buildServerThreadPoolConfig(serverConf, "server"))); } TlsConfig tlsDefaults = TlsUtils.extractTlsConfig(serverConf, CommonConstants.Server.SERVER_TLS_PREFIX); @@ -147,7 +150,7 @@ public final class ListenerConfigUtil { if (listeners.isEmpty()) { listeners.add( new ListenerConfig(CommonConstants.HTTP_PROTOCOL, DEFAULT_HOST, CommonConstants.Server.DEFAULT_ADMIN_API_PORT, - CommonConstants.HTTP_PROTOCOL, new TlsConfig())); + CommonConstants.HTTP_PROTOCOL, new TlsConfig(), buildServerThreadPoolConfig(serverConf, "server"))); } return listeners; @@ -159,7 +162,7 @@ public final class ListenerConfigUtil { String portString = minionConf.getProperty(CommonConstants.Helix.KEY_OF_MINION_PORT); if (portString != null) { listeners.add(new ListenerConfig(CommonConstants.HTTP_PROTOCOL, DEFAULT_HOST, Integer.parseInt(portString), - CommonConstants.HTTP_PROTOCOL, new TlsConfig())); + CommonConstants.HTTP_PROTOCOL, new TlsConfig(), buildServerThreadPoolConfig(minionConf, "minion"))); } TlsConfig tlsDefaults = TlsUtils.extractTlsConfig(minionConf, CommonConstants.Minion.MINION_TLS_PREFIX); @@ -169,7 +172,7 @@ public final class ListenerConfigUtil { if (listeners.isEmpty()) { listeners.add( new ListenerConfig(CommonConstants.HTTP_PROTOCOL, DEFAULT_HOST, CommonConstants.Minion.DEFAULT_HELIX_PORT, - CommonConstants.HTTP_PROTOCOL, new TlsConfig())); + CommonConstants.HTTP_PROTOCOL, new TlsConfig(), buildServerThreadPoolConfig(minionConf, "minion"))); } return listeners; @@ -182,7 +185,8 @@ public final class ListenerConfigUtil { return new ListenerConfig(name, getHost(config.getProperty(protocolNamespace + ".host", DEFAULT_HOST)), getPort(config.getProperty(protocolNamespace + ".port")), getProtocol(config.getProperty(protocolNamespace + ".protocol"), name), - TlsUtils.extractTlsConfig(config, protocolNamespace + ".tls", tlsConfig)); + TlsUtils.extractTlsConfig(config, protocolNamespace + ".tls", tlsConfig), + buildServerThreadPoolConfig(config, namespace)); } private static String getHost(String configuredHost) { @@ -231,7 +235,9 @@ public final class ListenerConfigUtil { listener.getTransport().getWorkerThreadPoolConfig().setThreadFactory( new ThreadFactoryBuilder().setNameFormat("grizzly-http-server-%d") - .setUncaughtExceptionHandler(new JerseyProcessingUncaughtExceptionHandler()).build()); + .setUncaughtExceptionHandler(new JerseyProcessingUncaughtExceptionHandler()).build()) + .setCorePoolSize(listenerConfig.getThreadPoolConfig().getCorePoolSize()) + .setMaxPoolSize(listenerConfig.getThreadPoolConfig().getMaxPoolSize()); if (CommonConstants.HTTPS_PROTOCOL.equals(listenerConfig.getProtocol())) { listener.setSecure(true); @@ -274,6 +280,21 @@ public final class ListenerConfigUtil { .setNeedClientAuth(tlsConfig.isClientAuthEnabled()).setEnabledProtocols(new String[]{"TLSv1.2"}); } + private static HttpServerThreadPoolConfig buildServerThreadPoolConfig(PinotConfiguration config, String namespace) { + String threadPoolNamespace = namespace + DOT_ACCESS_THREAD_POOL; + + HttpServerThreadPoolConfig threadPoolConfig = HttpServerThreadPoolConfig.defaultInstance(); + int corePoolSize = config.getProperty(threadPoolNamespace + "." + "corePoolSize", -1); + int maxPoolSize = config.getProperty(threadPoolNamespace + "." + "maxPoolSize", -1); + if (corePoolSize > 0) { + threadPoolConfig.setCorePoolSize(corePoolSize); + } + if (maxPoolSize > 0) { + threadPoolConfig.setMaxPoolSize(maxPoolSize); + } + return threadPoolConfig; + } + public static String toString(Collection<? extends ListenerConfig> listenerConfigs) { return StringUtils.join(listenerConfigs.stream() .map(listener -> String.format("%s://%s:%d", listener.getProtocol(), listener.getHost(), listener.getPort())) diff --git a/pinot-server/src/test/java/org/apache/pinot/server/api/AccessControlTest.java b/pinot-server/src/test/java/org/apache/pinot/server/api/AccessControlTest.java index 146b758f15..153fdf739a 100644 --- a/pinot-server/src/test/java/org/apache/pinot/server/api/AccessControlTest.java +++ b/pinot-server/src/test/java/org/apache/pinot/server/api/AccessControlTest.java @@ -33,6 +33,7 @@ import org.apache.commons.io.FileUtils; import org.apache.pinot.common.config.TlsConfig; import org.apache.pinot.common.metrics.ServerMetrics; import org.apache.pinot.core.auth.BasicAuthUtils; +import org.apache.pinot.core.transport.HttpServerThreadPoolConfig; import org.apache.pinot.core.transport.ListenerConfig; import org.apache.pinot.server.access.AccessControl; import org.apache.pinot.server.access.AccessControlFactory; @@ -87,7 +88,7 @@ public class AccessControlTest { int adminApiApplicationPort = getAvailablePort(); _adminApiApplication.start(Collections.singletonList( new ListenerConfig(CommonConstants.HTTP_PROTOCOL, "0.0.0.0", adminApiApplicationPort, - CommonConstants.HTTP_PROTOCOL, new TlsConfig()))); + CommonConstants.HTTP_PROTOCOL, new TlsConfig(), HttpServerThreadPoolConfig.defaultInstance()))); _webTarget = ClientBuilder.newClient().target( String.format("http://%s:%d", NetUtils.getHostAddress(), adminApiApplicationPort)); diff --git a/pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java b/pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java index 77288b8286..0b80cd0b64 100644 --- a/pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java +++ b/pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java @@ -37,6 +37,7 @@ import org.apache.pinot.common.utils.LLCSegmentName; import org.apache.pinot.core.data.manager.InstanceDataManager; import org.apache.pinot.core.data.manager.offline.OfflineTableDataManager; import org.apache.pinot.core.data.manager.realtime.SegmentUploader; +import org.apache.pinot.core.transport.HttpServerThreadPoolConfig; import org.apache.pinot.core.transport.ListenerConfig; import org.apache.pinot.segment.local.data.manager.TableDataManager; import org.apache.pinot.segment.local.data.manager.TableDataManagerConfig; @@ -139,7 +140,7 @@ public abstract class BaseResourceTest { _adminApiApplication = new AdminApiApplication(serverInstance, new AllowAllAccessFactory(), serverConf); _adminApiApplication.start(Collections.singletonList( new ListenerConfig(CommonConstants.HTTP_PROTOCOL, "0.0.0.0", CommonConstants.Server.DEFAULT_ADMIN_API_PORT, - CommonConstants.HTTP_PROTOCOL, new TlsConfig()))); + CommonConstants.HTTP_PROTOCOL, new TlsConfig(), HttpServerThreadPoolConfig.defaultInstance()))); _webTarget = ClientBuilder.newClient().target( String.format("http://%s:%d", NetUtils.getHostAddress(), CommonConstants.Server.DEFAULT_ADMIN_API_PORT)); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org