Revert "SOLR-17541: LBSolrClient implementations should agree on 'getClient()' semantics (#2899)"
This reverts commit 4c81ddc7305001fd986262f6b7a0dced09403127.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 1874714..74bcc09 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -71,10 +71,6 @@
* SOLR-17926: Improve tracking of time already spent to discount the limit for sub-requests when `timeAllowed` is used. (Andrzej Bialecki, hossman)
-* SOLR-17541: `LBHttp2SolrClient` now maintains a separate internal/delegate client per Solr Base URL. Both `LBHttp2SolrClient` and `CloudHttp2SolrClient`
- always create and manage these internal clients. The ability for callers to provide a pre-built client is removed. Callers may specify the internal client
- details by providing an instance of either `Http2SolrClient.Builder` or `HttpJdkSolrClient.Builder`. (James Dyer)
-
Optimizations
---------------------
* SOLR-17568: The CLI bin/solr export tool now contacts the appropriate nodes directly for data instead of proxying through one.
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
index 3ad1c64..688ce42 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -61,7 +61,6 @@
import org.apache.solr.client.solrj.cloud.SolrCloudManager;
import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
-import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder;
import org.apache.solr.client.solrj.impl.SolrClientCloudManager;
import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
@@ -964,11 +963,9 @@
if (cloudManager != null) {
return cloudManager;
}
-
cloudSolrClient =
new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader))
- .withInternalClientBuilder(
- new Http2SolrClient.Builder().withHttpClient(cc.getDefaultHttpSolrClient()))
+ .withHttpClient(cc.getDefaultHttpSolrClient())
.build();
cloudManager = new SolrClientCloudManager(cloudSolrClient, cc.getObjectCache());
}
diff --git a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
index e9631b2..2bf25a8 100644
--- a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
+++ b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
@@ -16,6 +16,7 @@
*/
package org.apache.solr.core;
+import java.util.List;
import java.util.concurrent.TimeUnit;
import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.common.util.IOUtils;
@@ -36,24 +37,22 @@
private final Http2SolrClient httpSolrClient;
- private final Http2SolrClient.Builder httpSolrClientBuilder;
-
private final InstrumentedHttpListenerFactory trackHttpSolrMetrics;
HttpSolrClientProvider(UpdateShardHandlerConfig cfg, SolrMetricsContext parentContext) {
trackHttpSolrMetrics = new InstrumentedHttpListenerFactory(getNameStrategy(cfg));
initializeMetrics(parentContext);
- this.httpSolrClientBuilder =
- new Http2SolrClient.Builder().addListenerFactory(trackHttpSolrMetrics);
+ Http2SolrClient.Builder httpClientBuilder =
+ new Http2SolrClient.Builder().withListenerFactory(List.of(trackHttpSolrMetrics));
if (cfg != null) {
- httpSolrClientBuilder
+ httpClientBuilder
.withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS)
.withIdleTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS)
.withMaxConnectionsPerHost(cfg.getMaxUpdateConnectionsPerHost());
}
- httpSolrClient = httpSolrClientBuilder.build();
+ httpSolrClient = httpClientBuilder.build();
}
private InstrumentedHttpListenerFactory.NameStrategy getNameStrategy(
@@ -77,7 +76,7 @@
}
void setSecurityBuilder(HttpClientBuilderPlugin builder) {
- builder.setup(httpSolrClientBuilder, httpSolrClient);
+ builder.setup(httpSolrClient);
}
@Override
diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
index c602898..b58da55 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
@@ -98,7 +98,7 @@
private final AtomicBoolean canceled = new AtomicBoolean(false);
private final Map<String, List<String>> shardToURLs;
- protected LBHttp2SolrClient<Http2SolrClient.Builder> lbClient;
+ protected LBHttp2SolrClient<Http2SolrClient> lbClient;
public HttpShardHandler(HttpShardHandlerFactory httpShardHandlerFactory) {
this.httpShardHandlerFactory = httpShardHandlerFactory;
diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
index 5fd2d70..0766367 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
@@ -82,9 +82,8 @@
protected ExecutorService commExecutor;
protected volatile Http2SolrClient defaultClient;
- protected Http2SolrClient.Builder httpSolrClientBuilder;
protected InstrumentedHttpListenerFactory httpListenerFactory;
- protected LBHttp2SolrClient<Http2SolrClient.Builder> loadbalancer;
+ protected LBHttp2SolrClient<Http2SolrClient> loadbalancer;
int corePoolSize = 0;
int maximumPoolSize = Integer.MAX_VALUE;
@@ -301,16 +300,15 @@
getParameter(
args, SolrHttpConstants.PROP_SO_TIMEOUT, SolrHttpConstants.DEFAULT_SO_TIMEOUT, sb);
- this.httpSolrClientBuilder =
+ this.defaultClient =
new Http2SolrClient.Builder()
.withConnectionTimeout(connectionTimeout, TimeUnit.MILLISECONDS)
.withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
.withExecutor(commExecutor)
.withMaxConnectionsPerHost(maxConnectionsPerHost)
- .addListenerFactory(this.httpListenerFactory);
- this.defaultClient = httpSolrClientBuilder.build();
-
- this.loadbalancer = new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder).build();
+ .build();
+ this.defaultClient.addListenerFactory(this.httpListenerFactory);
+ this.loadbalancer = new LBHttp2SolrClient.Builder<Http2SolrClient>(defaultClient).build();
initReplicaListTransformers(getParameter(args, "replicaRouting", null, sb));
@@ -320,7 +318,7 @@
@Override
public void setSecurityBuilder(HttpClientBuilderPlugin clientBuilderPlugin) {
if (clientBuilderPlugin != null) {
- clientBuilderPlugin.setup(httpSolrClientBuilder, defaultClient);
+ clientBuilderPlugin.setup(defaultClient);
}
}
diff --git a/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java b/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
index d26ae67..bc8495e 100644
--- a/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
@@ -27,9 +27,4 @@
public interface HttpClientBuilderPlugin {
public default void setup(Http2SolrClient client) {}
-
- /** TODO: Ideally, we only pass the builder here. */
- public default void setup(Http2SolrClient.Builder builder, Http2SolrClient client) {
- setup(client);
- }
}
diff --git a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
index e914448..3a70096 100644
--- a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
@@ -404,11 +404,6 @@
@Override
public void setup(Http2SolrClient client) {
- setup(null, client);
- }
-
- @Override
- public void setup(Http2SolrClient.Builder builder, Http2SolrClient client) {
final HttpListenerFactory.RequestResponseListener listener =
new HttpListenerFactory.RequestResponseListener() {
private static final String CACHED_REQUEST_USER_KEY = "cachedRequestUser";
@@ -465,12 +460,7 @@
(String) request.getAttributes().get(CACHED_REQUEST_USER_KEY));
}
};
- if (client != null) {
- client.addListenerFactory(() -> listener);
- }
- if (builder != null) {
- builder.addListenerFactory(() -> listener);
- }
+ client.addListenerFactory(() -> listener);
}
public boolean needsAuthorization(HttpServletRequest req) {
diff --git a/solr/core/src/test/org/apache/solr/cli/PostToolTest.java b/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
index 1edb469..7a37609 100644
--- a/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
+++ b/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
@@ -73,7 +73,6 @@
withBasicAuth(CollectionAdminRequest.createCollection(collection, "conf1", 1, 1, 0, 0))
.processAndWait(cluster.getSolrClient(), 10);
- waitForState("creating", collection, activeClusterShape(1, 1));
Path jsonDoc = Files.createTempFile("temp", ".json");
diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
index dedb455..770742c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
@@ -1932,16 +1932,17 @@
}
private SolrCloudManager getCloudDataProvider(ZkStateReader zkStateReader) {
- var httpSolrClientBuilder =
+ var httpSolrClient =
new Http2SolrClient.Builder()
.withIdleTimeout(30000, TimeUnit.MILLISECONDS)
- .withConnectionTimeout(15000, TimeUnit.MILLISECONDS);
+ .withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
+ .build();
var cloudSolrClient =
new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader))
- .withInternalClientBuilder(httpSolrClientBuilder)
+ .withHttpClient(httpSolrClient)
.build();
solrClients.add(cloudSolrClient);
- solrClients.add(httpSolrClientBuilder.build());
+ solrClients.add(httpSolrClient);
SolrClientCloudManager sccm = new SolrClientCloudManager(cloudSolrClient, null);
sccm.getClusterStateProvider().connect();
return sccm;
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
index 6824600..0adb494 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
@@ -18,11 +18,7 @@
import java.io.IOException;
-/**
- * A lambda intended for invoking SolrClient operations
- *
- * @lucene.experimental
- */
+/** A lambda intended for invoking SolrClient operations */
@FunctionalInterface
public interface SolrClientFunction<C extends SolrClient, R> {
R apply(C c) throws IOException, SolrServerException;
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
index 18aa318..ade1ebe 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
@@ -40,8 +40,9 @@
public class CloudHttp2SolrClient extends CloudSolrClient {
private final ClusterStateProvider stateProvider;
- private final LBHttp2SolrClient<Http2SolrClient.Builder> lbClient;
+ private final LBHttp2SolrClient<Http2SolrClient> lbClient;
private final Http2SolrClient myClient;
+ private final boolean clientIsInternal;
/**
* Create a new client object that connects to Zookeeper and is always aware of the SolrCloud
@@ -53,8 +54,8 @@
*/
protected CloudHttp2SolrClient(Builder builder) {
super(builder.shardLeadersOnly, builder.parallelUpdates, builder.directUpdatesToLeadersOnly);
- var httpSolrClientBuilder = createOrGetHttpClientBuilder(builder);
- this.myClient = httpSolrClientBuilder.build();
+ this.clientIsInternal = builder.httpClient == null;
+ this.myClient = createOrGetHttpClientFromBuilder(builder);
this.stateProvider = createClusterStateProvider(builder);
this.retryExpiryTimeNano = builder.retryExpiryTimeNano;
this.defaultCollection = builder.defaultCollection;
@@ -72,14 +73,16 @@
// locks.
this.locks = objectList(builder.parallelCacheRefreshesLocks);
- this.lbClient = new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder).build();
+ this.lbClient = new LBHttp2SolrClient.Builder<Http2SolrClient>(myClient).build();
}
- private Http2SolrClient.Builder createOrGetHttpClientBuilder(Builder builder) {
- if (builder.internalClientBuilder != null) {
- return builder.internalClientBuilder;
+ private Http2SolrClient createOrGetHttpClientFromBuilder(Builder builder) {
+ if (builder.httpClient != null) {
+ return builder.httpClient;
+ } else if (builder.internalClientBuilder != null) {
+ return builder.internalClientBuilder.build();
} else {
- return new Http2SolrClient.Builder();
+ return new Http2SolrClient.Builder().build();
}
}
@@ -126,7 +129,7 @@
private void closeMyClientIfNeeded() {
try {
- if (myClient != null) {
+ if (clientIsInternal && myClient != null) {
myClient.close();
}
} catch (Exception e) {
@@ -145,7 +148,7 @@
}
@Override
- public LBHttp2SolrClient<Http2SolrClient.Builder> getLbClient() {
+ public LBHttp2SolrClient<Http2SolrClient> getLbClient() {
return lbClient;
}
@@ -168,6 +171,7 @@
protected Collection<String> zkHosts = new ArrayList<>();
protected List<String> solrUrls = new ArrayList<>();
protected String zkChroot;
+ protected Http2SolrClient httpClient;
protected boolean shardLeadersOnly = true;
protected boolean directUpdatesToLeadersOnly = false;
protected boolean parallelUpdates = true;
@@ -401,6 +405,23 @@
}
/**
+ * Set the internal http client.
+ *
+ * <p>Note: closing the httpClient instance is at the responsibility of the caller.
+ *
+ * @param httpClient http client
+ * @return this
+ */
+ public Builder withHttpClient(Http2SolrClient httpClient) {
+ if (this.internalClientBuilder != null) {
+ throw new IllegalStateException(
+ "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided");
+ }
+ this.httpClient = httpClient;
+ return this;
+ }
+
+ /**
* If provided, the CloudHttp2SolrClient will build it's internal Http2SolrClient using this
* builder (instead of the empty default one). Providing this builder allows users to configure
* the internal clients (authentication, timeouts, etc.).
@@ -409,6 +430,10 @@
* @return this
*/
public Builder withInternalClientBuilder(Http2SolrClient.Builder internalClientBuilder) {
+ if (this.httpClient != null) {
+ throw new IllegalStateException(
+ "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided");
+ }
this.internalClientBuilder = internalClientBuilder;
return this;
}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index 60a206d..1fe2b91 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -154,8 +154,8 @@
}
// note: do not manipulate httpClient below this point; it could be a shared instance
- if (builder.listenerFactories != null) {
- this.listenerFactory.addAll(builder.listenerFactories);
+ if (builder.listenerFactory != null) {
+ this.listenerFactory.addAll(builder.listenerFactory);
}
updateDefaultMimeTypeForParser();
this.idleTimeoutMillis = builder.getIdleTimeoutMillis();
@@ -628,7 +628,6 @@
* @param clientFunction a Function that consumes a Http2SolrClient and returns an arbitrary value
* @return the value returned after invoking 'clientFunction'
* @param <R> the type returned by the provided function (and by this method)
- * @lucene.experimental
*/
public <R> R requestWithBaseUrl(
String baseUrl, SolrClientFunction<Http2SolrClient, R> clientFunction)
@@ -959,7 +958,7 @@
protected Long keyStoreReloadIntervalSecs;
- private List<HttpListenerFactory> listenerFactories = new ArrayList<>(0);
+ private List<HttpListenerFactory> listenerFactory;
public Builder() {
super();
@@ -985,27 +984,8 @@
this.baseSolrUrl = baseSolrUrl;
}
- /**
- * specify a listener factory, which will be appended to any existing values.
- *
- * @param listenerFactory a HttpListenerFactory
- * @return This Builder
- */
- public Http2SolrClient.Builder addListenerFactory(HttpListenerFactory listenerFactory) {
- this.listenerFactories.add(listenerFactory);
- return this;
- }
-
- /**
- * Specify listener factories, which will replace any existing values.
- *
- * @param listenerFactories a list of HttpListenerFactory instances
- * @return This Builder
- */
- public Http2SolrClient.Builder withListenerFactories(
- List<HttpListenerFactory> listenerFactories) {
- this.listenerFactories.clear();
- this.listenerFactories.addAll(listenerFactories);
+ public Http2SolrClient.Builder withListenerFactory(List<HttpListenerFactory> listenerFactory) {
+ this.listenerFactory = listenerFactory;
return this;
}
@@ -1123,8 +1103,8 @@
if (this.urlParamNames == null) {
this.urlParamNames = http2SolrClient.urlParamNames;
}
- if (this.listenerFactories == null) {
- this.listenerFactories = new ArrayList<>(http2SolrClient.listenerFactory);
+ if (this.listenerFactory == null) {
+ this.listenerFactory = new ArrayList<>(http2SolrClient.listenerFactory);
}
if (this.executor == null) {
this.executor = http2SolrClient.executor;
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
index 714c5b1..0bcecb2 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
@@ -145,7 +145,7 @@
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(
@@ -164,10 +164,10 @@
}
}
- @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());
@@ -199,13 +199,25 @@
}
}
- 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();
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index 1a796cf..28039a5 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -21,25 +21,22 @@
import java.net.SocketException;
import java.net.SocketTimeoutException;
import java.util.Arrays;
-import java.util.Collections;
-import java.util.Map;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.solr.client.solrj.ResponseParser;
+import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.common.SolrException;
-import org.apache.solr.common.util.IOUtils;
import org.apache.solr.common.util.NamedList;
import org.slf4j.MDC;
/**
- * This "LoadBalanced Http Solr Client" is a load balancer for multiple Http Solr Clients. 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.
@@ -57,7 +54,7 @@
* <blockquote>
*
* <pre>
- * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClientBuilder,
+ * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClient,
* new LBSolrClient.Endpoint("http://host1:8080/solr"), new LBSolrClient.Endpoint("http://host2:8080/solr"))
* .build();
* </pre>
@@ -70,7 +67,7 @@
* <blockquote>
*
* <pre>
- * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClientBuilder,
+ * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClient,
* new LBSolrClient.Endpoint("http://host1:8080/solr", "coreA"),
* new LBSolrClient.Endpoint("http://host2:8080/solr", "coreB"))
* .build();
@@ -95,63 +92,35 @@
*
* @since solr 8.0
*/
-public class LBHttp2SolrClient<B extends HttpSolrClientBuilderBase<?, ?>> extends LBSolrClient {
+public class LBHttp2SolrClient<C extends HttpSolrClientBase> extends LBSolrClient {
- private final Map<String, HttpSolrClientBase> urlToClient;
- private final Set<String> urlParamNames;
+ protected final C solrClient;
- // must synchronize on this when building
- private final HttpSolrClientBuilderBase<?, ?> solrClientBuilder;
-
+ @SuppressWarnings("unchecked")
private LBHttp2SolrClient(Builder<?> builder) {
super(Arrays.asList(builder.solrEndpoints));
- this.solrClientBuilder = builder.solrClientBuilder;
-
+ this.solrClient = (C) builder.solrClient;
this.aliveCheckIntervalMillis = builder.aliveCheckIntervalMillis;
this.defaultCollection = builder.defaultCollection;
-
- if (builder.solrClientBuilder.urlParamNames == null) {
- this.urlParamNames = Collections.emptySet();
- } else {
- this.urlParamNames = Set.copyOf(builder.solrClientBuilder.urlParamNames);
- }
-
- this.urlToClient = new ConcurrentHashMap<>();
- for (LBSolrClient.Endpoint endpoint : builder.solrEndpoints) {
- getClient(endpoint);
- }
}
@Override
- protected HttpSolrClientBase getClient(final Endpoint endpoint) {
- return urlToClient.computeIfAbsent(
- endpoint.getBaseUrl(),
- url -> {
- synchronized (solrClientBuilder) {
- solrClientBuilder.baseSolrUrl = url;
- return solrClientBuilder.build();
- }
- });
+ protected SolrClient getClient(Endpoint endpoint) {
+ return solrClient;
}
@Override
public ResponseParser getParser() {
- return urlToClient.isEmpty() ? null : urlToClient.values().iterator().next().getParser();
+ return solrClient.getParser();
}
@Override
public RequestWriter getRequestWriter() {
- return urlToClient.isEmpty() ? null : urlToClient.values().iterator().next().getRequestWriter();
+ return solrClient.getRequestWriter();
}
public Set<String> getUrlParamNames() {
- return urlParamNames;
- }
-
- @Override
- public void close() {
- urlToClient.values().forEach(IOUtils::closeQuietly);
- super.close();
+ return solrClient.getUrlParamNames();
}
/**
@@ -245,18 +214,23 @@
RetryListener listener) {
String baseUrl = endpoint.toString();
rsp.server = baseUrl;
- final var client = getClient(endpoint);
- CompletableFuture<NamedList<Object>> future =
- client.requestAsync(req.getRequest(), endpoint.getCore());
- future.whenComplete(
- (result, throwable) -> {
- if (!future.isCompletedExceptionally()) {
- onSuccessfulRequest(result, endpoint, rsp, isZombie, listener);
- } else if (!future.isCancelled()) {
- onFailedRequest(throwable, endpoint, isNonRetryable, isZombie, listener);
- }
- });
- return future;
+ final var client = (Http2SolrClient) getClient(endpoint);
+ try {
+ CompletableFuture<NamedList<Object>> future =
+ client.requestWithBaseUrl(baseUrl, (c) -> c.requestAsync(req.getRequest()));
+ future.whenComplete(
+ (result, throwable) -> {
+ if (!future.isCompletedExceptionally()) {
+ onSuccessfulRequest(result, endpoint, rsp, isZombie, listener);
+ } else if (!future.isCancelled()) {
+ onFailedRequest(throwable, endpoint, isNonRetryable, isZombie, listener);
+ }
+ });
+ return future;
+ } catch (SolrServerException | IOException e) {
+ // Unreachable, since 'requestWithBaseUrl' above is running the request asynchronously
+ throw new RuntimeException(e);
+ }
}
private void onSuccessfulRequest(
@@ -320,28 +294,16 @@
}
}
- public static class Builder<B extends HttpSolrClientBuilderBase<?, ?>> {
+ public static class Builder<C extends HttpSolrClientBase> {
- private final B solrClientBuilder;
-
+ 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;
- /**
- * Use this Builder to configure an LBHttp2SolrClient. The passed-in Solr Client Builder will be
- * used to generate an internal client per Endpoint.
- *
- * <p>Implementation Note: LBHttp2SolrClient will modify the passed-in Builder's {@link
- * HttpSolrClientBuilderBase#baseSolrUrl} whenever it needs to generate a new Http Solr Client.
- *
- * @param solrClientBuilder A Builder like {@link Http2SolrClient.Builder} used to generate the
- * internal clients
- * @param endpoints the initial Solr Endpoints to load balance
- */
- public Builder(B solrClientBuilder, Endpoint... endpoints) {
- this.solrClientBuilder = solrClientBuilder;
+ public Builder(C solrClient, Endpoint... endpoints) {
+ this.solrClient = solrClient;
this.solrEndpoints = endpoints;
}
@@ -351,7 +313,7 @@
*
* @param aliveCheckInterval how often to ping for aliveness
*/
- public Builder<B> 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);
@@ -361,13 +323,13 @@
}
/** Sets a default for core or collection based requests. */
- public Builder<B> withDefaultCollection(String defaultCoreOrCollection) {
+ public Builder<C> withDefaultCollection(String defaultCoreOrCollection) {
this.defaultCollection = defaultCoreOrCollection;
return this;
}
- public LBHttp2SolrClient<B> build() {
- return new LBHttp2SolrClient<B>(this);
+ public LBHttp2SolrClient<C> build() {
+ return new LBHttp2SolrClient<C>(this);
}
}
}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
index 132570b..85fadde 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
@@ -495,7 +495,25 @@
private NamedList<Object> doRequest(Endpoint endpoint, SolrRequest<?> solrRequest)
throws SolrServerException, IOException {
final var solrClient = getClient(endpoint);
- return solrClient.request(solrRequest, endpoint.getCore());
+ return doRequest(solrClient, endpoint.getBaseUrl(), endpoint.getCore(), solrRequest);
+ }
+
+ // TODO SOLR-17541 should remove the need for the special-casing below; remove as a part of that
+ // ticket.
+ private NamedList<Object> doRequest(
+ SolrClient solrClient, String baseUrl, String collection, SolrRequest<?> solrRequest)
+ throws SolrServerException, IOException {
+ // Some implementations of LBSolrClient.getClient(...) return a Http2SolrClient that may not be
+ // pointed at the desired URL (or any URL for that matter). We special case that here to ensure
+ // the appropriate URL is provided.
+ if (solrClient instanceof Http2SolrClient httpSolrClient) {
+ 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'
+ return solrClient.request(solrRequest, collection);
}
protected Exception doRequest(
@@ -604,7 +622,12 @@
// First the one on the endpoint, then the default collection
final String effectiveCollection =
Objects.requireNonNullElse(zombieEndpoint.getCore(), getDefaultCollection());
- final var responseRaw = getClient(zombieEndpoint).request(queryRequest, effectiveCollection);
+ final var responseRaw =
+ doRequest(
+ getClient(zombieEndpoint),
+ zombieEndpoint.getBaseUrl(),
+ effectiveCollection,
+ queryRequest);
QueryResponse resp = new QueryResponse();
resp.setResponse(responseRaw);
@@ -706,7 +729,7 @@
// Choose the endpoint's core/collection over any specified by the user
final var effectiveCollection =
endpoint.getCore() == null ? collection : endpoint.getCore();
- return getClient(endpoint).request(request, effectiveCollection);
+ return doRequest(getClient(endpoint), endpoint.getBaseUrl(), effectiveCollection, request);
} catch (SolrException e) {
// Server is alive but the request was malformed or invalid
throw e;
@@ -750,7 +773,8 @@
++numServersTried;
final String effectiveCollection =
endpoint.getCore() == null ? collection : endpoint.getCore();
- NamedList<Object> rsp = getClient(endpoint).request(request, effectiveCollection);
+ NamedList<Object> rsp =
+ doRequest(getClient(endpoint), endpoint.getBaseUrl(), effectiveCollection, request);
reviveZombieServer(wrapper.getEndpoint());
return rsp;
} catch (SolrException e) {
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java
index 46b6883..0846dfe 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java
@@ -121,6 +121,26 @@
}
@Test
+ public void testExternalClientAndInternalBuilderTogether() {
+ expectThrows(
+ IllegalStateException.class,
+ () ->
+ new CloudHttp2SolrClient.Builder(
+ Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT))
+ .withHttpClient(mock(Http2SolrClient.class))
+ .withInternalClientBuilder(mock(Http2SolrClient.Builder.class))
+ .build());
+ expectThrows(
+ IllegalStateException.class,
+ () ->
+ new CloudHttp2SolrClient.Builder(
+ Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT))
+ .withInternalClientBuilder(mock(Http2SolrClient.Builder.class))
+ .withHttpClient(mock(Http2SolrClient.class))
+ .build());
+ }
+
+ @Test
public void testProvideInternalBuilder() throws IOException {
Http2SolrClient http2Client = mock(Http2SolrClient.class);
Http2SolrClient.Builder http2ClientBuilder = mock(Http2SolrClient.Builder.class);
@@ -140,6 +160,20 @@
}
@Test
+ public void testProvideExternalClient() throws IOException {
+ Http2SolrClient http2Client = mock(Http2SolrClient.class);
+ CloudHttp2SolrClient.Builder clientBuilder =
+ new CloudHttp2SolrClient.Builder(
+ Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT))
+ .withHttpClient(http2Client);
+ try (CloudHttp2SolrClient client = clientBuilder.build()) {
+ assertEquals(http2Client, client.getHttpClient());
+ }
+ // it's external, should be NOT closed when closing CloudSolrClient
+ verify(http2Client, never()).close();
+ }
+
+ @Test
public void testDefaultCollectionPassedFromBuilderToClient() throws IOException {
try (CloudHttp2SolrClient createdClient =
new CloudHttp2SolrClient.Builder(
@@ -161,19 +195,29 @@
public void testHttpClientPreservedInHttp2ClusterStateProvider() throws IOException {
List<String> solrUrls = List.of(cluster.getJettySolrRunner(0).getBaseUrl().toString());
- // without internalClientBuilder
- testHttpClientConsistency(solrUrls, null);
+ // No httpClient - No internalClientBuilder
+ testHttpClientConsistency(solrUrls, null, null);
- // with internalClientBuilder
+ // httpClient - No internalClientBuilder
+ try (Http2SolrClient httpClient = new Http2SolrClient.Builder().build()) {
+ testHttpClientConsistency(solrUrls, httpClient, null);
+ }
+
+ // No httpClient - internalClientBuilder
Http2SolrClient.Builder internalClientBuilder = new Http2SolrClient.Builder();
- testHttpClientConsistency(solrUrls, internalClientBuilder);
+ testHttpClientConsistency(solrUrls, null, internalClientBuilder);
}
private void testHttpClientConsistency(
- List<String> solrUrls, Http2SolrClient.Builder internalClientBuilder) throws IOException {
+ List<String> solrUrls,
+ Http2SolrClient httpClient,
+ Http2SolrClient.Builder internalClientBuilder)
+ throws IOException {
CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder(solrUrls);
- if (internalClientBuilder != null) {
+ if (httpClient != null) {
+ clientBuilder.withHttpClient(httpClient);
+ } else if (internalClientBuilder != null) {
clientBuilder.withInternalClientBuilder(internalClientBuilder);
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
index 11fcad6..e6a4768 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
@@ -35,6 +35,7 @@
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.response.SolrPingResponse;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.MapSolrParams;
@@ -151,6 +152,26 @@
}
}
+ @Test
+ public void testRequestWithBaseUrl() throws Exception {
+ DebugServlet.clear();
+ DebugServlet.addResponseHeader("Content-Type", "application/octet-stream");
+ DebugServlet.responseBodyByQueryFragment.put("", javabinResponse());
+ String someOtherUrl = getBaseUrl() + "/some/other/base/url";
+ String intendedUrl = getBaseUrl() + DEBUG_SERVLET_PATH;
+ SolrQuery q = new SolrQuery("foo");
+ q.setParam("a", MUST_ENCODE);
+
+ HttpJdkSolrClient.Builder b =
+ builder(someOtherUrl).withResponseParser(new JavaBinResponseParser());
+ try (HttpJdkSolrClient client = b.build()) {
+ client.requestWithBaseUrl(intendedUrl, new QueryRequest(q, SolrRequest.METHOD.GET), null);
+ assertEquals(
+ client.getParser().getWriterType(), DebugServlet.parameters.get(CommonParams.WT)[0]);
+ }
+ }
+
+ @Test
public void testGetById() throws Exception {
DebugServlet.clear();
try (HttpJdkSolrClient client = builder(getBaseUrl() + DEBUG_SERVLET_PATH).build()) {
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java
index e9e6368..3f24984 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java
@@ -17,6 +17,7 @@
package org.apache.solr.client.solrj.impl;
import java.io.IOException;
+import java.io.UncheckedIOException;
import java.lang.invoke.MethodHandles;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -117,28 +118,30 @@
private LBClientHolder client(LBSolrClient.Endpoint... baseSolrEndpoints) {
if (random().nextBoolean()) {
- var delegateClientBuilder =
+ var delegateClient =
new Http2SolrClient.Builder()
.withConnectionTimeout(1000, TimeUnit.MILLISECONDS)
- .withIdleTimeout(2000, TimeUnit.MILLISECONDS);
+ .withIdleTimeout(2000, TimeUnit.MILLISECONDS)
+ .build();
var lbClient =
- new LBHttp2SolrClient.Builder<>(delegateClientBuilder, baseSolrEndpoints)
+ new LBHttp2SolrClient.Builder<>(delegateClient, baseSolrEndpoints)
.withDefaultCollection(solr[0].getDefaultCollection())
.setAliveCheckInterval(500, TimeUnit.MILLISECONDS)
.build();
- return new LBClientHolder(lbClient, delegateClientBuilder);
+ return new LBClientHolder(lbClient, delegateClient);
} else {
- var delegateClientBuilder =
+ var delegateClient =
new HttpJdkSolrClient.Builder()
.withConnectionTimeout(1000, TimeUnit.MILLISECONDS)
.withIdleTimeout(2000, TimeUnit.MILLISECONDS)
- .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT);
+ .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT)
+ .build();
var lbClient =
- new LBHttp2SolrClient.Builder<>(delegateClientBuilder, baseSolrEndpoints)
+ new LBHttp2SolrClient.Builder<>(delegateClient, baseSolrEndpoints)
.withDefaultCollection(solr[0].getDefaultCollection())
.setAliveCheckInterval(500, TimeUnit.MILLISECONDS)
.build();
- return new LBClientHolder(lbClient, delegateClientBuilder);
+ return new LBClientHolder(lbClient, delegateClient);
}
}
@@ -314,9 +317,9 @@
private static class LBClientHolder implements AutoCloseable {
final LBHttp2SolrClient<?> lbClient;
- final HttpSolrClientBuilderBase<?, ?> delegate;
+ final HttpSolrClientBase delegate;
- LBClientHolder(LBHttp2SolrClient<?> lbClient, HttpSolrClientBuilderBase<?, ?> delegate) {
+ LBClientHolder(LBHttp2SolrClient<?> lbClient, HttpSolrClientBase delegate) {
this.lbClient = lbClient;
this.delegate = delegate;
}
@@ -324,6 +327,11 @@
@Override
public void close() {
lbClient.close();
+ try {
+ delegate.close();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
}
}
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
index 30cee08..9d20193 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
@@ -18,8 +18,6 @@
import java.io.IOException;
import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
@@ -30,6 +28,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import org.apache.solr.SolrTestCase;
+import org.apache.solr.client.solrj.SolrClientFunction;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.request.QueryRequest;
@@ -52,12 +51,11 @@
Set<String> urlParamNames = new HashSet<>(2);
urlParamNames.add("param1");
- var httpSolrClientBuilder =
- new Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames);
- var endpoint = new LBSolrClient.Endpoint(url);
- try (var testClient =
- new LBHttp2SolrClient.Builder<Http2SolrClient.Builder>(httpSolrClientBuilder, endpoint)
- .build()) {
+ try (Http2SolrClient http2SolrClient =
+ new Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames).build();
+ LBHttp2SolrClient<Http2SolrClient> testClient =
+ new LBHttp2SolrClient.Builder<>(http2SolrClient, new LBSolrClient.Endpoint(url))
+ .build()) {
assertArrayEquals(
"Wrong urlParamNames found in lb client.",
@@ -66,7 +64,7 @@
assertArrayEquals(
"Wrong urlParamNames found in base client.",
urlParamNames.toArray(),
- testClient.getClient(endpoint).getUrlParamNames().toArray());
+ http2SolrClient.getUrlParamNames().toArray());
}
}
@@ -76,11 +74,12 @@
LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);
- var httpSolrClientBuilder =
- new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
-
- try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
- new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1, ep2).build()) {
+ Http2SolrClient.Builder b =
+ new Http2SolrClient.Builder("http://base.url").withConnectionTimeout(10, TimeUnit.SECONDS);
+ ;
+ try (MockHttpSolrClient client = new MockHttpSolrClient("http://base.url", b);
+ LBHttp2SolrClient<MockHttpSolrClient> testClient =
+ new LBHttp2SolrClient.Builder<>(client, ep1, ep2).build()) {
String lastEndpoint = null;
for (int i = 0; i < 10; i++) {
@@ -104,14 +103,15 @@
LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);
- var httpSolrClientBuilder =
- new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
+ Http2SolrClient.Builder b =
+ new Http2SolrClient.Builder("http://base.url").withConnectionTimeout(10, TimeUnit.SECONDS);
+ ;
+ try (MockHttpSolrClient client = new MockHttpSolrClient("http://base.url", b);
+ LBHttp2SolrClient<MockHttpSolrClient> testClient =
+ new LBHttp2SolrClient.Builder<>(client, ep1, ep2).build()) {
- try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
- new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1, ep2).build()) {
-
- setEndpointToFail(testClient, ep1);
- setEndpointToSucceed(testClient, ep2);
+ client.basePathToFail = ep1.getBaseUrl();
+ String basePathToSucceed = ep2.getBaseUrl();
String qValue = "First time";
for (int i = 0; i < 5; i++) {
@@ -121,13 +121,13 @@
LBSolrClient.Rsp response = testClient.request(req);
assertEquals(
"The healthy node 'endpoint two' should have served the request: " + i,
- ep2.getBaseUrl(),
+ basePathToSucceed,
response.server);
checkSynchonousResponseContent(response, qValue);
}
- setEndpointToFail(testClient, ep2);
- setEndpointToSucceed(testClient, ep1);
+ client.basePathToFail = ep2.getBaseUrl();
+ basePathToSucceed = ep1.getBaseUrl();
qValue = "Second time";
for (int i = 0; i < 5; i++) {
@@ -137,13 +137,21 @@
LBSolrClient.Rsp response = testClient.request(req);
assertEquals(
"The healthy node 'endpoint one' should have served the request: " + i,
- ep1.getBaseUrl(),
+ basePathToSucceed,
response.server);
checkSynchonousResponseContent(response, qValue);
}
}
}
+ private void checkSynchonousResponseContent(LBSolrClient.Rsp response, String qValue) {
+ assertEquals("There should be one element in the respnse.", 1, response.getResponse().size());
+ assertEquals(
+ "The response key 'response' should echo the query.",
+ qValue,
+ response.getResponse().get("response"));
+ }
+
@Test
public void testAsyncWithFailures() {
@@ -154,35 +162,28 @@
LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);
- var httpSolrClientBuilder =
- new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
-
- try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
- new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1, ep2).build()) {
+ Http2SolrClient.Builder b =
+ new Http2SolrClient.Builder("http://base.url").withConnectionTimeout(10, TimeUnit.SECONDS);
+ ;
+ try (MockHttpSolrClient client = new MockHttpSolrClient("http://base.url", b);
+ LBHttp2SolrClient<MockHttpSolrClient> testClient =
+ new LBHttp2SolrClient.Builder<>(client, ep1, ep2).build()) {
for (int j = 0; j < 2; j++) {
// first time Endpoint One will return error code 500.
// second time Endpoint One will be healthy
- LBSolrClient.Endpoint endpointToSucceed;
- LBSolrClient.Endpoint endpointToFail;
+ String basePathToSucceed;
if (j == 0) {
- setEndpointToFail(testClient, ep1);
- setEndpointToSucceed(testClient, ep2);
- endpointToSucceed = ep2;
- endpointToFail = ep1;
+ client.basePathToFail = ep1.getBaseUrl();
+ basePathToSucceed = ep2.getBaseUrl();
} else {
- setEndpointToFail(testClient, ep2);
- setEndpointToSucceed(testClient, ep1);
- endpointToSucceed = ep1;
- endpointToFail = ep2;
+ client.basePathToFail = ep2.getBaseUrl();
+ basePathToSucceed = ep1.getBaseUrl();
}
- List<String> successEndpointLastBasePaths =
- basePathsForEndpoint(testClient, endpointToSucceed);
- List<String> failEndpointLastBasePaths = basePathsForEndpoint(testClient, endpointToFail);
for (int i = 0; i < 10; i++) {
- // i: we'll try 10 times. It should behave the same with iter 2-10. .
+ // i: we'll try 10 times to see if it behaves the same every time.
QueryRequest queryRequest = new QueryRequest(new MapSolrParams(Map.of("q", "" + i)));
LBSolrClient.Req req = new LBSolrClient.Req(queryRequest, endpointList);
@@ -195,26 +196,26 @@
} catch (TimeoutException | ExecutionException e) {
fail(iterMessage + " Response ended in failure: " + e);
}
-
if (i == 0) {
- // When i=0, it must try both endpoints to find success:
+ // When j=0, "endpoint one" fails.
+ // The first time around (i) it tries the first, then the second.
//
- // with j=0, endpoint one is tried first because it
- // is first one the list, but it fails.
- // with j=1, endpoint two is tried first because
- // it is the only known healthy node, but
- // now it is failing.
- assertEquals(iterMessage, 1, successEndpointLastBasePaths.size());
- assertEquals(iterMessage, 1, failEndpointLastBasePaths.size());
+ // With j=0 and i>0, it only tries "endpoint two".
+ //
+ // When j=1 and i=0, "endpoint two" starts failing.
+ // So it tries both it and "endpoint one"
+ //
+ // With j=1 and i>0, it only tries "endpoint one".
+ assertEquals(iterMessage, 2, client.lastBasePaths.size());
+
+ String failedBasePath = client.lastBasePaths.remove(0);
+ assertEquals(iterMessage, client.basePathToFail, failedBasePath);
} else {
- // With i>0,
- // With j=0 and i>0, it only tries "endpoint two".
- // With j=1 and i>0, it only tries "endpoint one".
- assertEquals(iterMessage, 1, successEndpointLastBasePaths.size());
- assertTrue(iterMessage, failEndpointLastBasePaths.isEmpty());
+ // The first endpoint does not give the exception, it doesn't retry.
+ assertEquals(iterMessage, 1, client.lastBasePaths.size());
}
- successEndpointLastBasePaths.clear();
- failEndpointLastBasePaths.clear();
+ String successBasePath = client.lastBasePaths.remove(0);
+ assertEquals(iterMessage, basePathToSucceed, successBasePath);
}
}
}
@@ -226,11 +227,11 @@
LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);
- var httpSolrClientBuilder =
- new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
-
- try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
- new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1, ep2).build()) {
+ Http2SolrClient.Builder b =
+ new Http2SolrClient.Builder("http://base.url").withConnectionTimeout(10, TimeUnit.SECONDS);
+ try (MockHttpSolrClient client = new MockHttpSolrClient("http://base.url", b);
+ LBHttp2SolrClient<MockHttpSolrClient> testClient =
+ new LBHttp2SolrClient.Builder<>(client, ep1, ep2).build()) {
int limit = 10; // For simplicity use an even limit
List<CompletableFuture<LBSolrClient.Rsp>> responses = new ArrayList<>();
@@ -242,17 +243,23 @@
}
QueryRequest[] queryRequests = new QueryRequest[limit];
- List<SolrRequest<?>> lastSolrRequests = lastSolrRequests(testClient, ep1, ep2);
- assertEquals(limit, lastSolrRequests.size());
-
+ int numEndpointOne = 0;
+ int numEndpointTwo = 0;
for (int i = 0; i < limit; i++) {
- SolrRequest<?> lastSolrReq = lastSolrRequests.get(i);
+ SolrRequest<?> lastSolrReq = client.lastSolrRequests.get(i);
assertTrue(lastSolrReq instanceof QueryRequest);
QueryRequest lastQueryReq = (QueryRequest) lastSolrReq;
int index = Integer.parseInt(lastQueryReq.getParams().get("q"));
assertNull("Found same request twice: " + index, queryRequests[index]);
queryRequests[index] = lastQueryReq;
+ final String lastBasePath = client.lastBasePaths.get(i);
+ if (lastBasePath.equals(ep1.toString())) {
+ numEndpointOne++;
+ } else if (lastBasePath.equals(ep2.toString())) {
+ numEndpointTwo++;
+ }
+
LBSolrClient.Rsp lastRsp = null;
try {
lastRsp = responses.get(index).get();
@@ -271,55 +278,15 @@
// It is the user's responsibility to shuffle the endpoints when using
// async. LB Http Solr Client will always try the passed-in endpoints
// in order. In this case, endpoint 1 gets all the requests!
- List<String> ep1BasePaths = basePathsForEndpoint(testClient, ep1);
- List<String> ep2BasePaths = basePathsForEndpoint(testClient, ep2);
- assertEquals(limit, basePathsForEndpoint(testClient, ep1).size());
- assertEquals(0, basePathsForEndpoint(testClient, ep2).size());
+ assertEquals(limit, numEndpointOne);
+ assertEquals(0, numEndpointTwo);
+
+ assertEquals(limit, client.lastSolrRequests.size());
+ assertEquals(limit, client.lastCollections.size());
}
}
- private void checkSynchonousResponseContent(LBSolrClient.Rsp response, String qValue) {
- assertEquals("There should be one element in the response.", 1, response.getResponse().size());
- assertEquals(
- "The response key 'response' should echo the query.",
- qValue,
- response.getResponse().get("response"));
- }
-
- private void setEndpointToFail(
- LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient, LBSolrClient.Endpoint ep) {
- ((MockHttpSolrClient) testClient.getClient(ep)).allRequestsShallFail = true;
- }
-
- private void setEndpointToSucceed(
- LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient, LBSolrClient.Endpoint ep) {
- ((MockHttpSolrClient) testClient.getClient(ep)).allRequestsShallFail = false;
- }
-
- private List<String> basePathsForEndpoint(
- LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient, LBSolrClient.Endpoint ep) {
- return ((MockHttpSolrClient) testClient.getClient(ep)).lastBasePaths;
- }
-
- private List<SolrRequest<?>> lastSolrRequests(
- LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient, LBSolrClient.Endpoint... endpoints) {
- return Arrays.stream(endpoints)
- .map(testClient::getClient)
- .map(MockHttpSolrClient.class::cast)
- .flatMap(c -> c.lastSolrRequests.stream())
- .toList();
- }
-
- public static class MockHttpSolrClientBuilder
- extends HttpSolrClientBuilderBase<MockHttpSolrClientBuilder, MockHttpSolrClient> {
-
- @Override
- public MockHttpSolrClient build() {
- return new MockHttpSolrClient(baseSolrUrl, this);
- }
- }
-
- public static class MockHttpSolrClient extends HttpSolrClientBase {
+ public static class MockHttpSolrClient extends Http2SolrClient {
public List<SolrRequest<?>> lastSolrRequests = new ArrayList<>();
@@ -327,13 +294,15 @@
public List<String> lastCollections = new ArrayList<>();
- public boolean allRequestsShallFail;
+ public String basePathToFail = null;
public String tmpBaseUrl = null;
- public boolean closeCalled;
+ protected MockHttpSolrClient(String serverBaseUrl, Builder builder) {
- protected MockHttpSolrClient(String serverBaseUrl, MockHttpSolrClientBuilder builder) {
+ // TODO: Consider creating an interface for Http*SolrClient
+ // so mocks can Implement, not Extend, and not actually need to
+ // build an (unused) client
super(serverBaseUrl, builder);
}
@@ -343,20 +312,33 @@
lastSolrRequests.add(request);
lastBasePaths.add(tmpBaseUrl);
lastCollections.add(collection);
- if (allRequestsShallFail) {
+ if (tmpBaseUrl.equals(basePathToFail)) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "We should retry this.");
}
return generateResponse(request);
}
@Override
+ public <R> R requestWithBaseUrl(
+ String baseUrl, SolrClientFunction<Http2SolrClient, R> clientFunction)
+ throws SolrServerException, IOException {
+ // This use of 'tmpBaseUrl' is NOT thread safe, but that's fine for our purposes here.
+ try {
+ tmpBaseUrl = baseUrl;
+ return clientFunction.apply(this);
+ } finally {
+ tmpBaseUrl = null;
+ }
+ }
+
+ @Override
public CompletableFuture<NamedList<Object>> requestAsync(
final SolrRequest<?> solrRequest, String collection) {
CompletableFuture<NamedList<Object>> cf = new CompletableFuture<>();
lastSolrRequests.add(solrRequest);
lastBasePaths.add(tmpBaseUrl);
lastCollections.add(collection);
- if (allRequestsShallFail) {
+ if (tmpBaseUrl != null && tmpBaseUrl.equals(basePathToFail)) {
cf.completeExceptionally(
new SolrException(SolrException.ErrorCode.SERVER_ERROR, "We should retry this."));
} else {
@@ -369,32 +351,5 @@
String id = solrRequest.getParams().get("q");
return new NamedList<>(Collections.singletonMap("response", id));
}
-
- @Override
- public void close() throws IOException {
- closeCalled = true;
- }
-
- @Override
- protected boolean isFollowRedirects() {
- return false;
- }
-
- @Override
- protected boolean processorAcceptsMimeType(
- Collection<String> processorSupportedContentTypes, String mimeType) {
- return false;
- }
-
- @Override
- protected String allProcessorSupportedContentTypesCommaDelimited(
- Collection<String> processorSupportedContentTypes) {
- return null;
- }
-
- @Override
- protected void updateDefaultMimeTypeForParser() {
- // no-op
- }
}
}
