Skip to content

Commit d0000b4

Browse files
iamsanjaydsmiley
iamsanjay
and
dsmiley
authored
Switched from HTTP1 to HTTP2 in SolrClientCloudManager (#2751)
* Replaced CloudLegacySolrClient with CloudHttp2SolrClient in SolrClientCloudManager and updated all internal classes accordingly. * SolrCloudManager#httpRequest is removed. * Removed dependencies on Apache httpclient and httpcore from solrj-zookeeper --------- Co-authored-by: David Smiley <[email protected]>
1 parent d840a3b commit d0000b4

File tree

10 files changed

+88
-163
lines changed

10 files changed

+88
-163
lines changed

‎solr/CHANGES.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,8 @@ Optimizations
170170

171171
* SOLR-17441: Improve system metrics collection by skipping unreadable MXBean properties, making /admin/info/system calls faster (Haythem Khiri)
172172

173+
* SOLR-16503: Switched from HTTP1 to HTTP2 in SolrClientCloudManager by replacing CloudLegacySolrClient with CloudHttp2SolrClient. (Sanjay Dutt, David Smiley)
174+
173175
Bug Fixes
174176
---------------------
175177
* SOLR-12429: Uploading a configset with a symbolic link produces a IOException. Now a error message to user generated instead. (Eric Pugh)

‎solr/core/src/java/org/apache/solr/cloud/ZkController.java‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@
5454
import java.util.stream.Collectors;
5555
import org.apache.solr.client.solrj.SolrClient;
5656
import org.apache.solr.client.solrj.cloud.SolrCloudManager;
57-
import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;
57+
import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient;
58+
import org.apache.solr.client.solrj.impl.Http2SolrClient;
5859
import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder;
5960
import org.apache.solr.client.solrj.impl.SolrClientCloudManager;
6061
import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
61-
import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
6262
import org.apache.solr.client.solrj.request.CoreAdminRequest.WaitForState;
6363
import org.apache.solr.cloud.overseer.ClusterStateMutator;
6464
import org.apache.solr.cloud.overseer.OverseerAction;
@@ -199,7 +199,11 @@ public String toString() {
199199
private final SolrZkClient zkClient;
200200
public final ZkStateReader zkStateReader;
201201
private SolrCloudManager cloudManager;
202-
private CloudLegacySolrClient cloudSolrClient;
202+
203+
// only for internal usage
204+
private Http2SolrClient http2SolrClient;
205+
206+
private CloudHttp2SolrClient cloudSolrClient;
203207

204208
private final String zkServerAddress; // example: 127.0.0.1:54062/solr
205209

@@ -768,8 +772,9 @@ public void close() {
768772
} finally {
769773

770774
sysPropsCacher.close();
771-
customThreadPool.execute(() -> IOUtils.closeQuietly(cloudSolrClient));
772775
customThreadPool.execute(() -> IOUtils.closeQuietly(cloudManager));
776+
customThreadPool.execute(() -> IOUtils.closeQuietly(cloudSolrClient));
777+
customThreadPool.execute(() -> IOUtils.closeQuietly(http2SolrClient));
773778

774779
try {
775780
try {
@@ -865,11 +870,15 @@ public SolrCloudManager getSolrCloudManager() {
865870
if (cloudManager != null) {
866871
return cloudManager;
867872
}
868-
cloudSolrClient =
869-
new CloudLegacySolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader))
870-
.withHttpClient(cc.getUpdateShardHandler().getDefaultHttpClient())
873+
http2SolrClient =
874+
new Http2SolrClient.Builder()
875+
.withHttpClient(cc.getDefaultHttpSolrClient())
876+
.withIdleTimeout(30000, TimeUnit.MILLISECONDS)
871877
.withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
872-
.withSocketTimeout(30000, TimeUnit.MILLISECONDS)
878+
.build();
879+
cloudSolrClient =
880+
new CloudHttp2SolrClient.Builder(List.of(getZkServerAddress()), Optional.empty())
881+
.withHttpClient(http2SolrClient)
873882
.build();
874883
cloudManager = new SolrClientCloudManager(cloudSolrClient, cc.getObjectCache());
875884
cloudManager.getClusterStateProvider().connect();

‎solr/core/src/test/org/apache/solr/cloud/OverseerTest.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@
5252
import org.apache.solr.client.solrj.SolrClient;
5353
import org.apache.solr.client.solrj.cloud.DistributedQueue;
5454
import org.apache.solr.client.solrj.cloud.SolrCloudManager;
55-
import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;
56-
import org.apache.solr.client.solrj.impl.CloudSolrClient;
55+
import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient;
56+
import org.apache.solr.client.solrj.impl.Http2SolrClient;
5757
import org.apache.solr.client.solrj.impl.SolrClientCloudManager;
5858
import org.apache.solr.cloud.overseer.NodeMutator;
5959
import org.apache.solr.cloud.overseer.OverseerAction;
@@ -127,7 +127,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
127127
Collections.synchronizedList(new ArrayList<>());
128128
private final List<UpdateShardHandler> updateShardHandlers =
129129
Collections.synchronizedList(new ArrayList<>());
130-
private final List<CloudSolrClient> solrClients = Collections.synchronizedList(new ArrayList<>());
130+
private final List<SolrClient> solrClients = Collections.synchronizedList(new ArrayList<>());
131131
private static final String COLLECTION = SolrTestCaseJ4.DEFAULT_TEST_COLLECTION_NAME;
132132

133133
public static class MockZKController {
@@ -1948,13 +1948,18 @@ public Void answer(InvocationOnMock invocation) {
19481948
}
19491949

19501950
private SolrCloudManager getCloudDataProvider(String zkAddress) {
1951-
var client =
1952-
new CloudLegacySolrClient.Builder(Collections.singletonList(zkAddress), Optional.empty())
1953-
.withSocketTimeout(30000, TimeUnit.MILLISECONDS)
1951+
var httpSolrClient =
1952+
new Http2SolrClient.Builder()
1953+
.withIdleTimeout(30000, TimeUnit.MILLISECONDS)
19541954
.withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
19551955
.build();
1956-
solrClients.add(client);
1957-
SolrClientCloudManager sccm = new SolrClientCloudManager(client);
1956+
var cloudSolrClient =
1957+
new CloudHttp2SolrClient.Builder(Collections.singletonList(zkAddress), Optional.empty())
1958+
.withHttpClient(httpSolrClient)
1959+
.build();
1960+
solrClients.add(cloudSolrClient);
1961+
solrClients.add(httpSolrClient);
1962+
SolrClientCloudManager sccm = new SolrClientCloudManager(cloudSolrClient, null);
19581963
sccm.getClusterStateProvider().connect();
19591964
return sccm;
19601965
}

‎solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.concurrent.atomic.AtomicInteger;
3737
import java.util.concurrent.atomic.AtomicReference;
3838
import org.apache.solr.SolrTestCaseJ4;
39+
import org.apache.solr.client.solrj.impl.Http2SolrClient;
3940
import org.apache.solr.common.MapWriter;
4041
import org.apache.solr.common.cloud.ClusterProperties;
4142
import org.apache.solr.common.cloud.ClusterState;
@@ -514,6 +515,7 @@ public void tearDown() throws Exception {
514515
private static class MockCoreContainer extends CoreContainer {
515516
UpdateShardHandler updateShardHandler =
516517
new UpdateShardHandler(UpdateShardHandlerConfig.DEFAULT);
518+
Http2SolrClient solrClient;
517519

518520
public MockCoreContainer() {
519521
super(SolrXmlConfig.fromString(TEST_PATH(), "<solr/>"));
@@ -522,6 +524,7 @@ public MockCoreContainer() {
522524
this.shardHandlerFactory = httpShardHandlerFactory;
523525
this.coreAdminHandler = new CoreAdminHandler();
524526
this.metricManager = mock(SolrMetricManager.class);
527+
this.solrClient = new Http2SolrClient.Builder().build();
525528
}
526529

527530
@Override
@@ -535,9 +538,15 @@ public UpdateShardHandler getUpdateShardHandler() {
535538
@Override
536539
public void shutdown() {
537540
updateShardHandler.close();
541+
solrClient.close();
538542
super.shutdown();
539543
}
540544

545+
@Override
546+
public Http2SolrClient getDefaultHttpSolrClient() {
547+
return solrClient;
548+
}
549+
541550
@Override
542551
public SolrMetricManager getMetricManager() {
543552
return metricManager;

‎solr/solrj-zookeeper/build.gradle‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ dependencies {
3434

3535
// declare dependencies we use even though already declared by solrj-core
3636
implementation 'org.slf4j:slf4j-api'
37-
implementation 'org.apache.httpcomponents:httpclient'
38-
implementation 'org.apache.httpcomponents:httpcore'
3937

4038
api('org.apache.zookeeper:zookeeper', {
4139
exclude group: "org.apache.yetus", module: "audience-annotations"

‎solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/cloud/DelegatingCloudManager.java‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.apache.solr.client.solrj.cloud;
1818

1919
import java.io.IOException;
20-
import java.util.Map;
2120
import org.apache.solr.client.solrj.SolrRequest;
2221
import org.apache.solr.client.solrj.SolrResponse;
2322
import org.apache.solr.client.solrj.impl.ClusterStateProvider;
@@ -69,18 +68,6 @@ public <T extends SolrResponse> T request(SolrRequest<T> req) throws IOException
6968
return delegate.request(req);
7069
}
7170

72-
@Override
73-
public byte[] httpRequest(
74-
String url,
75-
SolrRequest.METHOD method,
76-
Map<String, String> headers,
77-
String payload,
78-
int timeout,
79-
boolean followRedirects)
80-
throws IOException {
81-
return delegate.httpRequest(url, method, headers, payload, timeout, followRedirects);
82-
}
83-
8471
@Override
8572
public void close() throws IOException {
8673
delegate.close();

‎solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/cloud/SolrCloudManager.java‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package org.apache.solr.client.solrj.cloud;
1919

2020
import java.io.IOException;
21-
import java.util.Map;
2221
import org.apache.solr.client.solrj.SolrRequest;
2322
import org.apache.solr.client.solrj.SolrResponse;
2423
import org.apache.solr.client.solrj.impl.ClusterStateProvider;
@@ -53,13 +52,4 @@ default ClusterState getClusterState() throws IOException {
5352
// Solr-like methods
5453

5554
<T extends SolrResponse> T request(SolrRequest<T> req) throws IOException;
56-
57-
byte[] httpRequest(
58-
String url,
59-
SolrRequest.METHOD method,
60-
Map<String, String> headers,
61-
String payload,
62-
int timeout,
63-
boolean followRedirects)
64-
throws IOException;
6555
}

‎solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java‎

Lines changed: 7 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,6 @@
1919

2020
import java.io.IOException;
2121
import java.lang.invoke.MethodHandles;
22-
import java.nio.charset.StandardCharsets;
23-
import java.util.Map;
24-
import org.apache.http.HttpEntity;
25-
import org.apache.http.HttpResponse;
26-
import org.apache.http.client.HttpClient;
27-
import org.apache.http.client.config.RequestConfig;
28-
import org.apache.http.client.methods.HttpDelete;
29-
import org.apache.http.client.methods.HttpGet;
30-
import org.apache.http.client.methods.HttpPost;
31-
import org.apache.http.client.methods.HttpPut;
32-
import org.apache.http.client.methods.HttpRequestBase;
33-
import org.apache.http.client.protocol.HttpClientContext;
34-
import org.apache.http.entity.StringEntity;
35-
import org.apache.http.util.EntityUtils;
3622
import org.apache.solr.client.solrj.SolrRequest;
3723
import org.apache.solr.client.solrj.SolrResponse;
3824
import org.apache.solr.client.solrj.SolrServerException;
@@ -51,21 +37,17 @@
5137
public class SolrClientCloudManager implements SolrCloudManager {
5238
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
5339

54-
protected final CloudLegacySolrClient solrClient;
40+
private final CloudHttp2SolrClient cloudSolrClient;
5541
private final ZkDistribStateManager stateManager;
5642
private final ZkStateReader zkStateReader;
5743
private final SolrZkClient zkClient;
5844
private final ObjectCache objectCache;
5945
private final boolean closeObjectCache;
6046
private volatile boolean isClosed;
6147

62-
public SolrClientCloudManager(CloudLegacySolrClient solrClient) {
63-
this(solrClient, null);
64-
}
65-
66-
public SolrClientCloudManager(CloudLegacySolrClient solrClient, ObjectCache objectCache) {
67-
this.solrClient = solrClient;
68-
this.zkStateReader = ZkStateReader.from(solrClient);
48+
public SolrClientCloudManager(CloudHttp2SolrClient client, ObjectCache objectCache) {
49+
this.cloudSolrClient = client;
50+
this.zkStateReader = ZkStateReader.from(client);
6951
this.zkClient = zkStateReader.getZkClient();
7052
this.stateManager = new ZkDistribStateManager(zkClient);
7153
this.isClosed = false;
@@ -103,12 +85,12 @@ public TimeSource getTimeSource() {
10385

10486
@Override
10587
public ClusterStateProvider getClusterStateProvider() {
106-
return solrClient.getClusterStateProvider();
88+
return cloudSolrClient.getClusterStateProvider();
10789
}
10890

10991
@Override
11092
public NodeStateProvider getNodeStateProvider() {
111-
return new SolrClientNodeStateProvider(solrClient);
93+
return new SolrClientNodeStateProvider(cloudSolrClient);
11294
}
11395

11496
@Override
@@ -119,76 +101,14 @@ public DistribStateManager getDistribStateManager() {
119101
@Override
120102
public <T extends SolrResponse> T request(SolrRequest<T> req) throws IOException {
121103
try {
122-
return req.process(solrClient);
104+
return req.process(cloudSolrClient);
123105
} catch (SolrServerException e) {
124106
throw new IOException(e);
125107
}
126108
}
127109

128110
private static final byte[] EMPTY = new byte[0];
129111

130-
@Override
131-
public byte[] httpRequest(
132-
String url,
133-
SolrRequest.METHOD method,
134-
Map<String, String> headers,
135-
String payload,
136-
int timeout,
137-
boolean followRedirects)
138-
throws IOException {
139-
HttpClient client = solrClient.getHttpClient();
140-
final HttpRequestBase req;
141-
HttpEntity entity = null;
142-
if (payload != null) {
143-
entity = new StringEntity(payload, StandardCharsets.UTF_8);
144-
}
145-
switch (method) {
146-
case GET:
147-
req = new HttpGet(url);
148-
break;
149-
case POST:
150-
req = new HttpPost(url);
151-
if (entity != null) {
152-
((HttpPost) req).setEntity(entity);
153-
}
154-
break;
155-
case PUT:
156-
req = new HttpPut(url);
157-
if (entity != null) {
158-
((HttpPut) req).setEntity(entity);
159-
}
160-
break;
161-
case DELETE:
162-
req = new HttpDelete(url);
163-
break;
164-
default:
165-
throw new IOException("Unsupported method " + method);
166-
}
167-
if (headers != null) {
168-
headers.forEach((k, v) -> req.addHeader(k, v));
169-
}
170-
RequestConfig.Builder requestConfigBuilder = HttpClientUtil.createDefaultRequestConfigBuilder();
171-
if (timeout > 0) {
172-
requestConfigBuilder.setSocketTimeout(timeout);
173-
requestConfigBuilder.setConnectTimeout(timeout);
174-
}
175-
requestConfigBuilder.setRedirectsEnabled(followRedirects);
176-
req.setConfig(requestConfigBuilder.build());
177-
HttpClientContext httpClientRequestContext = HttpClientUtil.createNewHttpClientRequestContext();
178-
HttpResponse rsp = client.execute(req, httpClientRequestContext);
179-
int statusCode = rsp.getStatusLine().getStatusCode();
180-
if (statusCode != 200) {
181-
throw new IOException(
182-
"Error sending request to " + url + ", HTTP response: " + rsp.toString());
183-
}
184-
HttpEntity responseEntity = rsp.getEntity();
185-
if (responseEntity != null && responseEntity.getContent() != null) {
186-
return EntityUtils.toByteArray(responseEntity);
187-
} else {
188-
return EMPTY;
189-
}
190-
}
191-
192112
public SolrZkClient getZkClient() {
193113
return zkClient;
194114
}

0 commit comments

Comments
 (0)