Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17516: LBHttpSolrClient: support HttpJdkSolrClient (Generic Version) #2828

Merged
merged 26 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c7e306c
Add additional unit tests
jdyer1 Oct 28, 2024
eae31a7
Both test classes to use same naming convention and in same package
jdyer1 Oct 28, 2024
ec026e2
- Randomly use either http client delegate in integration test
jdyer1 Oct 28, 2024
87bf82d
Use Self Signed Cert-Friendy SSL Context in Integration Test
jdyer1 Oct 31, 2024
898977a
Make LBHttp2SolrClient Generic
jdyer1 Oct 31, 2024
5200329
cleanup
jdyer1 Oct 31, 2024
025a6c0
tidy
jdyer1 Oct 31, 2024
392cde2
add license header
jdyer1 Oct 31, 2024
565cfad
cleanup
jdyer1 Oct 31, 2024
4c4c975
Merge branch 'main' into feature/SOLR-17516-c
jdyer1 Oct 31, 2024
2aba104
javadoc adjustment
jdyer1 Oct 31, 2024
8096039
tidy
jdyer1 Oct 31, 2024
b241c10
Small JavaDoc improvement
jdyer1 Nov 1, 2024
9df2df7
tidy
jdyer1 Nov 1, 2024
2d3449d
- LBSolrClient to catch treat IOException same as if wrapped in SolrS…
jdyer1 Nov 4, 2024
0f040b2
tidy
jdyer1 Nov 4, 2024
c6d3509
Merge branch 'main' into feature/SOLR-17516-c
jdyer1 Nov 5, 2024
0526351
refactoring to allow jdk client to partcipate in changes
jdyer1 Nov 6, 2024
6db4f75
Revert "refactoring to allow jdk client to partcipate in changes"
jdyer1 Nov 7, 2024
be8c973
Merge branch 'main' into feature/SOLR-17516-c
jdyer1 Nov 14, 2024
311d85a
fix test after merge
jdyer1 Nov 14, 2024
d725daf
Exception for HttpJdkSolrClient in LBSolrClient
jdyer1 Nov 15, 2024
f3bc8e0
tidy
jdyer1 Nov 15, 2024
551d03c
add missing override
jdyer1 Nov 15, 2024
01d139f
Merge branch 'main' into feature/SOLR-17516-c
jdyer1 Nov 19, 2024
037c994
CHANGES.txt
jdyer1 Nov 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ Improvements

* SOLR-17495: Change Solr CLI delete command to not delete configs by default. Decouple lifecycle of collections from configsets. (Eric Pugh)

* SOLR-17516: `LBHttp2SolrClient` is now generic, adding support for `HttpJdkSolrClient`. (James Dyer)

Optimizations
---------------------
(No changes)
Expand Down Expand Up @@ -71,7 +73,7 @@ Deprecation Removals
in WordBreakSolrSpellChecker (Andrey Bozhko via Eric Pugh)

* SOLR-14763: Remove deprecated asynchronous request methods from `Http2SolrClient`, `HttpJdkSolrClient` and `LBHttp2SolrClient`
in favor of the new CompletableFuture based methods. Remove the related deprecated interfaces `AsyncListener` and ``Cancellable`
in favor of the new CompletableFuture based methods. Remove the related deprecated interfaces `AsyncListener` and `Cancellable`
(James Dyer)

* SOLR-14115: Remove deprecated zkcli script in favour of equivalent bin/solr sub commmands. (Eric Pugh)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import net.jcip.annotations.NotThreadSafe;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.client.solrj.impl.LBHttp2SolrClient;
import org.apache.solr.client.solrj.impl.LBSolrClient;
import org.apache.solr.client.solrj.request.QueryRequest;
Expand Down Expand Up @@ -113,7 +114,7 @@ public class HttpShardHandler extends ShardHandler {
protected AtomicInteger pending;

private final Map<String, List<String>> shardToURLs;
protected LBHttp2SolrClient lbClient;
protected LBHttp2SolrClient<Http2SolrClient> lbClient;

public HttpShardHandler(HttpShardHandlerFactory httpShardHandlerFactory) {
this.httpShardHandlerFactory = httpShardHandlerFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public class HttpShardHandlerFactory extends ShardHandlerFactory

protected volatile Http2SolrClient defaultClient;
protected InstrumentedHttpListenerFactory httpListenerFactory;
protected LBHttp2SolrClient loadbalancer;
protected LBHttp2SolrClient<Http2SolrClient> loadbalancer;

int corePoolSize = 0;
int maximumPoolSize = Integer.MAX_VALUE;
Expand Down Expand Up @@ -314,7 +314,7 @@ public void init(PluginInfo info) {
.withMaxConnectionsPerHost(maxConnectionsPerHost)
.build();
this.defaultClient.addListenerFactory(this.httpListenerFactory);
this.loadbalancer = new LBHttp2SolrClient.Builder(defaultClient).build();
this.loadbalancer = new LBHttp2SolrClient.Builder<Http2SolrClient>(defaultClient).build();

initReplicaListTransformers(getParameter(args, "replicaRouting", null, sb));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
public class CloudHttp2SolrClient extends CloudSolrClient {

private final ClusterStateProvider stateProvider;
private final LBHttp2SolrClient lbClient;
private final LBHttp2SolrClient<Http2SolrClient> lbClient;
private final Http2SolrClient myClient;
private final boolean clientIsInternal;

Expand Down Expand Up @@ -73,7 +73,7 @@ protected CloudHttp2SolrClient(Builder builder) {
// locks.
this.locks = objectList(builder.parallelCacheRefreshesLocks);

this.lbClient = new LBHttp2SolrClient.Builder(myClient).build();
this.lbClient = new LBHttp2SolrClient.Builder<Http2SolrClient>(myClient).build();
}

private Http2SolrClient createOrGetHttpClientFromBuilder(Builder builder) {
Expand Down Expand Up @@ -149,7 +149,7 @@ public void close() throws IOException {
}

@Override
public LBHttp2SolrClient getLbClient() {
public LBHttp2SolrClient<Http2SolrClient> getLbClient() {
return lbClient;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -831,10 +831,6 @@ protected String allProcessorSupportedContentTypesCommaDelimited(
.collect(Collectors.joining(", "));
}

protected RequestWriter getRequestWriter() {
return requestWriter;
}

/**
* An Http2SolrClient that doesn't close or cleanup any resources
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ protected HttpJdkSolrClient(String serverBaseUrl, HttpJdkSolrClient.Builder buil
public CompletableFuture<NamedList<Object>> requestAsync(
final SolrRequest<?> solrRequest, String collection) {
try {
PreparedRequest pReq = prepareRequest(solrRequest, collection);
PreparedRequest pReq = prepareRequest(solrRequest, collection, null);
return httpClient
.sendAsync(pReq.reqb.build(), HttpResponse.BodyHandlers.ofInputStream())
.thenApply(
Expand All @@ -157,10 +157,10 @@ public CompletableFuture<NamedList<Object>> requestAsync(
}
}

@Override
public NamedList<Object> request(SolrRequest<?> solrRequest, String collection)
protected NamedList<Object> requestWithBaseUrl(
String baseUrl, SolrRequest<?> solrRequest, String collection)
throws SolrServerException, IOException {
PreparedRequest pReq = prepareRequest(solrRequest, collection);
PreparedRequest pReq = prepareRequest(solrRequest, collection, baseUrl);
HttpResponse<InputStream> response = null;
try {
response = httpClient.send(pReq.reqb.build(), HttpResponse.BodyHandlers.ofInputStream());
Expand All @@ -173,8 +173,8 @@ public NamedList<Object> request(SolrRequest<?> solrRequest, String collection)
"Timeout occurred while waiting response from server at: " + pReq.url, e);
} catch (SolrException se) {
throw se;
} catch (RuntimeException re) {
throw new SolrServerException(re);
} catch (RuntimeException e) {
throw new SolrServerException(e);
} finally {
if (pReq.contentWritingFuture != null) {
pReq.contentWritingFuture.cancel(true);
Expand All @@ -192,13 +192,25 @@ public NamedList<Object> request(SolrRequest<?> solrRequest, String collection)
}
}

private PreparedRequest prepareRequest(SolrRequest<?> solrRequest, String collection)
@Override
public NamedList<Object> request(SolrRequest<?> solrRequest, String collection)
throws SolrServerException, IOException {
return requestWithBaseUrl(null, solrRequest, collection);
}

private PreparedRequest prepareRequest(
SolrRequest<?> solrRequest, String collection, String overrideBaseUrl)
throws SolrServerException, IOException {
checkClosed();
if (ClientUtils.shouldApplyDefaultCollection(collection, solrRequest)) {
collection = defaultCollection;
}
String url = getRequestUrl(solrRequest, collection);
String url;
if (overrideBaseUrl != null) {
url = ClientUtils.buildRequestUrl(solrRequest, overrideBaseUrl, collection);
} else {
url = getRequestUrl(solrRequest, collection);
}
ResponseParser parserToUse = responseParser(solrRequest);
ModifiableSolrParams queryParams = initializeSolrParams(solrRequest, parserToUse);
var reqb = HttpRequest.newBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ protected ResponseParser responseParser(SolrRequest<?> solrRequest) {
return solrRequest.getResponseParser() == null ? this.parser : solrRequest.getResponseParser();
}

protected RequestWriter getRequestWriter() {
return requestWriter;
}

// TODO: Remove this for 10.0, there is a typo in the method name
@Deprecated(since = "9.8", forRemoval = true)
protected ModifiableSolrParams initalizeSolrParams(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@
import org.slf4j.MDC;

/**
* LBHttp2SolrClient or "LoadBalanced LBHttp2SolrClient" is a load balancing wrapper around {@link
* Http2SolrClient}. This is useful when you have multiple Solr endpoints and requests need to be
* Load Balanced among them.
* This "LoadBalanced Http Solr Client" is a load balancing wrapper around a Http Solr Client. This
* is useful when you have multiple Solr endpoints and requests need to be Load Balanced among them.
*
* <p>Do <b>NOT</b> use this class for indexing in leader/follower scenarios since documents must be
* sent to the correct leader; no inter-node routing is done.
Expand Down Expand Up @@ -95,12 +94,14 @@
*
* @since solr 8.0
*/
public class LBHttp2SolrClient extends LBSolrClient {
private final Http2SolrClient solrClient;
public class LBHttp2SolrClient<C extends HttpSolrClientBase> extends LBSolrClient {

private LBHttp2SolrClient(Builder builder) {
protected final C solrClient;

@SuppressWarnings("unchecked")
private LBHttp2SolrClient(Builder<?> builder) {
super(Arrays.asList(builder.solrEndpoints));
this.solrClient = builder.http2SolrClient;
this.solrClient = (C) builder.solrClient;
this.aliveCheckIntervalMillis = builder.aliveCheckIntervalMillis;
this.defaultCollection = builder.defaultCollection;
}
Expand Down Expand Up @@ -289,16 +290,16 @@ private void onFailedRequest(
}
}

public static class Builder {
public static class Builder<C extends HttpSolrClientBase> {

private final Http2SolrClient http2SolrClient;
private final Endpoint[] solrEndpoints;
private final C solrClient;
private final LBSolrClient.Endpoint[] solrEndpoints;
private long aliveCheckIntervalMillis =
TimeUnit.MILLISECONDS.convert(60, TimeUnit.SECONDS); // 1 minute between checks
protected String defaultCollection;

public Builder(Http2SolrClient http2Client, Endpoint... endpoints) {
this.http2SolrClient = http2Client;
public Builder(C solrClient, Endpoint... endpoints) {
this.solrClient = solrClient;
this.solrEndpoints = endpoints;
}

Expand All @@ -308,7 +309,7 @@ public Builder(Http2SolrClient http2Client, Endpoint... endpoints) {
*
* @param aliveCheckInterval how often to ping for aliveness
*/
public LBHttp2SolrClient.Builder setAliveCheckInterval(int aliveCheckInterval, TimeUnit unit) {
public Builder<C> setAliveCheckInterval(int aliveCheckInterval, TimeUnit unit) {
if (aliveCheckInterval <= 0) {
throw new IllegalArgumentException(
"Alive check interval must be " + "positive, specified value = " + aliveCheckInterval);
Expand All @@ -318,13 +319,13 @@ public LBHttp2SolrClient.Builder setAliveCheckInterval(int aliveCheckInterval, T
}

/** Sets a default for core or collection based requests. */
public LBHttp2SolrClient.Builder withDefaultCollection(String defaultCoreOrCollection) {
public Builder<C> withDefaultCollection(String defaultCoreOrCollection) {
this.defaultCollection = defaultCoreOrCollection;
return this;
}

public LBHttp2SolrClient build() {
return new LBHttp2SolrClient(this);
public LBHttp2SolrClient<C> build() {
return new LBHttp2SolrClient<C>(this);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,8 @@ private NamedList<Object> doRequest(
if (solrClient instanceof Http2SolrClient) {
final var httpSolrClient = (Http2SolrClient) solrClient;
return httpSolrClient.requestWithBaseUrl(baseUrl, (c) -> c.request(solrRequest, collection));
} else if (solrClient instanceof HttpJdkSolrClient) {
return ((HttpJdkSolrClient) solrClient).requestWithBaseUrl(baseUrl, solrRequest, collection);
}

// Assume provided client already uses 'baseUrl'
Expand Down Expand Up @@ -730,11 +732,20 @@ public NamedList<Object> request(
if (e.getRootCause() instanceof IOException) {
ex = e;
moveAliveToDead(wrapper);
if (justFailed == null) justFailed = new HashMap<>();
if (justFailed == null) {
justFailed = new HashMap<>();
}
Comment on lines -733 to +737
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a shame to see duplication in getRootCause IOException detection with the dedicated clause. Maybe it'd be cleaner to have another try-catch just around calling request() above that detects the IOException and throws it? Just a thought; I leave it to you.

justFailed.put(endpoint.getUrl(), wrapper);
} else {
throw e;
}
} catch (IOException e) {
ex = e;
moveAliveToDead(wrapper);
if (justFailed == null) {
justFailed = new HashMap<>();
}
justFailed.put(endpoint.getUrl(), wrapper);
} catch (Exception e) {
throw new SolrServerException(e);
}
Expand Down
Loading
Loading