This is an automated email from the ASF dual-hosted git repository.
jackie 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 2050376adc Fix Bottleneck for Server Bootstrap by Making
maxConnsPerRoute Configurable (#10487)
2050376adc is described below
commit 2050376adcb67041972d9707b338373a5e6bc468
Author: Ankit Sultana <[email protected]>
AuthorDate: Fri Mar 31 04:05:22 2023 +0530
Fix Bottleneck for Server Bootstrap by Making maxConnsPerRoute Configurable
(#10487)
---
.../common/utils/FileUploadDownloadClient.java | 11 ++-
.../common/utils/fetcher/HttpSegmentFetcher.java | 3 +-
.../common/utils/fetcher/HttpsSegmentFetcher.java | 3 +-
.../apache/pinot/common/utils/http/HttpClient.java | 22 +++++-
.../pinot/common/utils/http/HttpClientConfig.java | 92 ++++++++++++++++++++++
.../common/utils/http/HttpClientConfigTest.java | 43 ++++++++++
6 files changed, 167 insertions(+), 7 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java
index f78e3b4818..c369843a61 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java
@@ -57,6 +57,7 @@ import org.apache.pinot.common.auth.AuthProviderUtils;
import org.apache.pinot.common.exception.HttpErrorStatusException;
import org.apache.pinot.common.restlet.resources.StartReplaceSegmentsRequest;
import org.apache.pinot.common.utils.http.HttpClient;
+import org.apache.pinot.common.utils.http.HttpClientConfig;
import org.apache.pinot.spi.auth.AuthProvider;
import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.utils.CommonConstants;
@@ -129,8 +130,16 @@ public class FileUploadDownloadClient implements
AutoCloseable {
_httpClient = new HttpClient();
}
+ public FileUploadDownloadClient(HttpClientConfig httpClientConfig) {
+ _httpClient = new HttpClient(httpClientConfig, null);
+ }
+
public FileUploadDownloadClient(SSLContext sslContext) {
- _httpClient = new HttpClient(sslContext);
+ _httpClient = new HttpClient(HttpClientConfig.DEFAULT_HTTP_CLIENT_CONFIG,
sslContext);
+ }
+
+ public FileUploadDownloadClient(HttpClientConfig httpClientConfig,
SSLContext sslContext) {
+ _httpClient = new HttpClient(httpClientConfig, sslContext);
}
public HttpClient getHttpClient() {
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/HttpSegmentFetcher.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/HttpSegmentFetcher.java
index 741ac18315..8dc6162fd0 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/HttpSegmentFetcher.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/HttpSegmentFetcher.java
@@ -32,6 +32,7 @@ import org.apache.http.message.BasicHeader;
import org.apache.pinot.common.exception.HttpErrorStatusException;
import org.apache.pinot.common.utils.FileUploadDownloadClient;
import org.apache.pinot.common.utils.RoundRobinURIProvider;
+import org.apache.pinot.common.utils.http.HttpClientConfig;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.retry.RetryPolicies;
@@ -41,7 +42,7 @@ public class HttpSegmentFetcher extends BaseSegmentFetcher {
@Override
protected void doInit(PinotConfiguration config) {
- _httpClient = new FileUploadDownloadClient();
+ _httpClient = new
FileUploadDownloadClient(HttpClientConfig.newBuilder(config).build());
}
@Override
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/HttpsSegmentFetcher.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/HttpsSegmentFetcher.java
index 64dcb601c1..71528595d8 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/HttpsSegmentFetcher.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/HttpsSegmentFetcher.java
@@ -22,6 +22,7 @@ import java.util.Set;
import javax.net.ssl.SSLContext;
import org.apache.pinot.common.utils.ClientSSLContextGenerator;
import org.apache.pinot.common.utils.FileUploadDownloadClient;
+import org.apache.pinot.common.utils.http.HttpClientConfig;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.CommonConstants;
@@ -69,6 +70,6 @@ public class HttpsSegmentFetcher extends HttpSegmentFetcher {
}
SSLContext sslContext = new
ClientSSLContextGenerator(sslConfig).generate();
- _httpClient = new FileUploadDownloadClient(sslContext);
+ _httpClient = new
FileUploadDownloadClient(HttpClientConfig.newBuilder(config).build(),
sslContext);
}
}
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java
index 4bd7117ab8..6a74512aae 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java
@@ -53,6 +53,7 @@ import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.entity.mime.MultipartEntityBuilder;
import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.client.HttpClients;
import org.apache.http.util.EntityUtils;
import org.apache.pinot.common.auth.AuthProviderUtils;
@@ -86,14 +87,14 @@ public class HttpClient implements AutoCloseable {
private final CloseableHttpClient _httpClient;
public HttpClient() {
- this(null);
+ this(HttpClientConfig.DEFAULT_HTTP_CLIENT_CONFIG, null);
}
- public HttpClient(@Nullable SSLContext sslContext) {
+ public HttpClient(HttpClientConfig httpClientConfig, @Nullable SSLContext
sslContext) {
SSLContext context = sslContext != null ? sslContext :
TlsUtils.getSslContext();
// Set NoopHostnameVerifier to skip validating hostname when
uploading/downloading segments.
SSLConnectionSocketFactory csf = new SSLConnectionSocketFactory(context,
NoopHostnameVerifier.INSTANCE);
- _httpClient = HttpClients.custom().setSSLSocketFactory(csf).build();
+ _httpClient = buildCloseableHttpClient(httpClientConfig, csf);
}
public static HttpClient getInstance() {
@@ -101,7 +102,8 @@ public class HttpClient implements AutoCloseable {
}
private static final class HttpClientHolder {
- static final HttpClient HTTP_CLIENT = new
HttpClient(TlsUtils.getSslContext());
+ static final HttpClient HTTP_CLIENT =
+ new HttpClient(HttpClientConfig.DEFAULT_HTTP_CLIENT_CONFIG,
TlsUtils.getSslContext());
}
// --------------------------------------------------------------------------
@@ -464,6 +466,18 @@ public class HttpClient implements AutoCloseable {
requestBuilder.setConfig(requestConfig);
}
+ private static CloseableHttpClient buildCloseableHttpClient(HttpClientConfig
httpClientConfig,
+ SSLConnectionSocketFactory csf) {
+ HttpClientBuilder httpClientBuilder =
HttpClients.custom().setSSLSocketFactory(csf);
+ if (httpClientConfig.getMaxConnTotal() > 0) {
+ httpClientBuilder.setMaxConnTotal(httpClientConfig.getMaxConnTotal());
+ }
+ if (httpClientConfig.getMaxConnPerRoute() > 0) {
+
httpClientBuilder.setMaxConnPerRoute(httpClientConfig.getMaxConnPerRoute());
+ }
+ return httpClientBuilder.build();
+ }
+
private static String getErrorMessage(HttpUriRequest request,
CloseableHttpResponse response) {
String controllerHost = null;
String controllerVersion = null;
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClientConfig.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClientConfig.java
new file mode 100644
index 0000000000..d7eedf5380
--- /dev/null
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClientConfig.java
@@ -0,0 +1,92 @@
+/**
+ * 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.common.utils.http;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.pinot.spi.env.PinotConfiguration;
+
+
+public class HttpClientConfig {
+ // Default config uses default values which are same as what Apache Commons
Http-Client uses.
+ public static final HttpClientConfig DEFAULT_HTTP_CLIENT_CONFIG =
HttpClientConfig.newBuilder().build();
+
+ protected static final String MAX_CONNS_CONFIG_NAME =
"http.client.maxConnTotal";
+ protected static final String MAX_CONNS_PER_ROUTE_CONFIG_NAME =
"http.client.maxConnPerRoute";
+
+ private final int _maxConnTotal;
+ private final int _maxConnPerRoute;
+
+ private HttpClientConfig(int maxConnTotal, int maxConnPerRoute) {
+ _maxConnTotal = maxConnTotal;
+ _maxConnPerRoute = maxConnPerRoute;
+ }
+
+ public int getMaxConnTotal() {
+ return _maxConnTotal;
+ }
+
+ public int getMaxConnPerRoute() {
+ return _maxConnPerRoute;
+ }
+
+ /**
+ * Creates a {@link HttpClientConfig.Builder} and initializes it with
relevant configs from the provided
+ * configuration. Since http-clients are used in a bunch of places in the
code, each use-case can have their own
+ * prefix for their config. The caller should call {@link
PinotConfiguration#subset(String)} to remove their prefix
+ * and this builder will look for exact matches of its relevant configs.
+ */
+ public static Builder newBuilder(PinotConfiguration pinotConfiguration) {
+ Builder builder = new Builder();
+ String maxConns = pinotConfiguration.getProperty(MAX_CONNS_CONFIG_NAME);
+ if (StringUtils.isNotEmpty(maxConns)) {
+ builder.withMaxConns(Integer.parseInt(maxConns));
+ }
+ String maxConnsPerRoute =
pinotConfiguration.getProperty(MAX_CONNS_PER_ROUTE_CONFIG_NAME);
+ if (StringUtils.isNotEmpty(maxConnsPerRoute)) {
+ builder.withMaxConnsPerRoute(Integer.parseInt(maxConnsPerRoute));
+ }
+ return builder;
+ }
+
+ private static Builder newBuilder() {
+ return new Builder();
+ }
+
+ public static class Builder {
+ private int _maxConns = -1;
+ private int _maxConnsPerRoute = -1;
+
+ private Builder() {
+ }
+
+ public Builder withMaxConns(int maxConns) {
+ _maxConns = maxConns;
+ return this;
+ }
+
+ public Builder withMaxConnsPerRoute(int maxConnsPerRoute) {
+ _maxConnsPerRoute = maxConnsPerRoute;
+ return this;
+ }
+
+ public HttpClientConfig build() {
+ return new HttpClientConfig(_maxConns, _maxConnsPerRoute);
+ }
+ }
+}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/utils/http/HttpClientConfigTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/utils/http/HttpClientConfigTest.java
new file mode 100644
index 0000000000..3cccac0f0e
--- /dev/null
+++
b/pinot-common/src/test/java/org/apache/pinot/common/utils/http/HttpClientConfigTest.java
@@ -0,0 +1,43 @@
+/**
+ * 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.common.utils.http;
+
+import org.apache.pinot.spi.env.PinotConfiguration;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class HttpClientConfigTest {
+
+ @Test
+ public void testNewBuilder() {
+ // Ensure config values are picked up by the builder.
+ PinotConfiguration pinotConfiguration = new PinotConfiguration();
+ pinotConfiguration.setProperty(HttpClientConfig.MAX_CONNS_CONFIG_NAME,
"123");
+
pinotConfiguration.setProperty(HttpClientConfig.MAX_CONNS_PER_ROUTE_CONFIG_NAME,
"11");
+ HttpClientConfig httpClientConfig =
HttpClientConfig.newBuilder(pinotConfiguration).build();
+ Assert.assertEquals(123, httpClientConfig.getMaxConnTotal());
+ Assert.assertEquals(11, httpClientConfig.getMaxConnPerRoute());
+
+ // Ensure default builder uses negative values
+ HttpClientConfig defaultConfig = HttpClientConfig.newBuilder(new
PinotConfiguration()).build();
+ Assert.assertTrue(defaultConfig.getMaxConnTotal() < 0, "default value
should be < 0");
+ Assert.assertTrue(defaultConfig.getMaxConnPerRoute() < 0, "default value
should be < 0");
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
