This is an automated email from the ASF dual-hosted git repository.
jdyer pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 4c81ddc7305 SOLR-17541: LBSolrClient implementations should agree on
'getClient()' semantics (#2899)
4c81ddc7305 is described below
commit 4c81ddc7305001fd986262f6b7a0dced09403127
Author: jdyer1 <[email protected]>
AuthorDate: Tue Sep 30 08:08:38 2025 -0500
SOLR-17541: LBSolrClient implementations should agree on 'getClient()'
semantics (#2899)
---
solr/CHANGES.txt | 4 +
.../java/org/apache/solr/cloud/ZkController.java | 5 +-
.../apache/solr/core/HttpSolrClientProvider.java | 13 +-
.../solr/handler/component/HttpShardHandler.java | 2 +-
.../handler/component/HttpShardHandlerFactory.java | 14 +-
.../solr/security/HttpClientBuilderPlugin.java | 5 +
.../solr/security/PKIAuthenticationPlugin.java | 12 +-
.../src/test/org/apache/solr/cli/PostToolTest.java | 1 +
.../test/org/apache/solr/cloud/OverseerTest.java | 9 +-
.../solr/client/solrj/SolrClientFunction.java | 6 +-
.../client/solrj/impl/CloudHttp2SolrClient.java | 45 +---
.../solr/client/solrj/impl/Http2SolrClient.java | 34 ++-
.../solr/client/solrj/impl/HttpJdkSolrClient.java | 24 +-
.../solr/client/solrj/impl/LBHttp2SolrClient.java | 116 ++++++----
.../solr/client/solrj/impl/LBSolrClient.java | 32 +--
.../impl/CloudHttp2SolrClientBuilderTest.java | 56 +----
.../client/solrj/impl/HttpJdkSolrClientTest.java | 21 --
.../impl/LBHttp2SolrClientIntegrationTest.java | 28 +--
.../client/solrj/impl/LBHttp2SolrClientTest.java | 247 ++++++++++++---------
19 files changed, 336 insertions(+), 338 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 74bcc09b0ab..1874714b55f 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -71,6 +71,10 @@ Improvements
* 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 688ce428501..3ad1c64c4eb 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -61,6 +61,7 @@ import org.apache.solr.client.solrj.SolrClient;
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;
@@ -963,9 +964,11 @@ public class ZkController implements Closeable {
if (cloudManager != null) {
return cloudManager;
}
+
cloudSolrClient =
new CloudHttp2SolrClient.Builder(new
ZkClientClusterStateProvider(zkStateReader))
- .withHttpClient(cc.getDefaultHttpSolrClient())
+ .withInternalClientBuilder(
+ new
Http2SolrClient.Builder().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 2bf25a896f6..e9631b26d1f 100644
--- a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
+++ b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
@@ -16,7 +16,6 @@
*/
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;
@@ -37,22 +36,24 @@ final class HttpSolrClientProvider implements AutoCloseable
{
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);
- Http2SolrClient.Builder httpClientBuilder =
- new
Http2SolrClient.Builder().withListenerFactory(List.of(trackHttpSolrMetrics));
+ this.httpSolrClientBuilder =
+ new Http2SolrClient.Builder().addListenerFactory(trackHttpSolrMetrics);
if (cfg != null) {
- httpClientBuilder
+ httpSolrClientBuilder
.withConnectionTimeout(cfg.getDistributedConnectionTimeout(),
TimeUnit.MILLISECONDS)
.withIdleTimeout(cfg.getDistributedSocketTimeout(),
TimeUnit.MILLISECONDS)
.withMaxConnectionsPerHost(cfg.getMaxUpdateConnectionsPerHost());
}
- httpSolrClient = httpClientBuilder.build();
+ httpSolrClient = httpSolrClientBuilder.build();
}
private InstrumentedHttpListenerFactory.NameStrategy getNameStrategy(
@@ -76,7 +77,7 @@ final class HttpSolrClientProvider implements AutoCloseable {
}
void setSecurityBuilder(HttpClientBuilderPlugin builder) {
- builder.setup(httpSolrClient);
+ builder.setup(httpSolrClientBuilder, 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 b58da55859a..c602898544d 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 @@ public class HttpShardHandler extends ShardHandler {
private final AtomicBoolean canceled = new AtomicBoolean(false);
private final Map<String, List<String>> shardToURLs;
- protected LBHttp2SolrClient<Http2SolrClient> lbClient;
+ protected LBHttp2SolrClient<Http2SolrClient.Builder> 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 07663679058..5fd2d7064e2 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,8 +82,9 @@ public class HttpShardHandlerFactory extends
ShardHandlerFactory
protected ExecutorService commExecutor;
protected volatile Http2SolrClient defaultClient;
+ protected Http2SolrClient.Builder httpSolrClientBuilder;
protected InstrumentedHttpListenerFactory httpListenerFactory;
- protected LBHttp2SolrClient<Http2SolrClient> loadbalancer;
+ protected LBHttp2SolrClient<Http2SolrClient.Builder> loadbalancer;
int corePoolSize = 0;
int maximumPoolSize = Integer.MAX_VALUE;
@@ -300,15 +301,16 @@ public class HttpShardHandlerFactory extends
ShardHandlerFactory
getParameter(
args, SolrHttpConstants.PROP_SO_TIMEOUT,
SolrHttpConstants.DEFAULT_SO_TIMEOUT, sb);
- this.defaultClient =
+ this.httpSolrClientBuilder =
new Http2SolrClient.Builder()
.withConnectionTimeout(connectionTimeout, TimeUnit.MILLISECONDS)
.withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
.withExecutor(commExecutor)
.withMaxConnectionsPerHost(maxConnectionsPerHost)
- .build();
- this.defaultClient.addListenerFactory(this.httpListenerFactory);
- this.loadbalancer = new
LBHttp2SolrClient.Builder<Http2SolrClient>(defaultClient).build();
+ .addListenerFactory(this.httpListenerFactory);
+ this.defaultClient = httpSolrClientBuilder.build();
+
+ this.loadbalancer = new
LBHttp2SolrClient.Builder<>(httpSolrClientBuilder).build();
initReplicaListTransformers(getParameter(args, "replicaRouting", null,
sb));
@@ -318,7 +320,7 @@ public class HttpShardHandlerFactory extends
ShardHandlerFactory
@Override
public void setSecurityBuilder(HttpClientBuilderPlugin clientBuilderPlugin) {
if (clientBuilderPlugin != null) {
- clientBuilderPlugin.setup(defaultClient);
+ clientBuilderPlugin.setup(httpSolrClientBuilder, 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 bc8495e9416..d26ae672c3f 100644
--- a/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
@@ -27,4 +27,9 @@ import org.apache.solr.client.solrj.impl.Http2SolrClient;
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 3a7009682fd..e9144488583 100644
--- a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
@@ -404,6 +404,11 @@ public class PKIAuthenticationPlugin extends
AuthenticationPlugin
@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";
@@ -460,7 +465,12 @@ public class PKIAuthenticationPlugin extends
AuthenticationPlugin
(String) request.getAttributes().get(CACHED_REQUEST_USER_KEY));
}
};
- client.addListenerFactory(() -> listener);
+ if (client != null) {
+ client.addListenerFactory(() -> listener);
+ }
+ if (builder != null) {
+ builder.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 7a3760992a3..1edb4695e3d 100644
--- a/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
+++ b/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
@@ -73,6 +73,7 @@ public class PostToolTest extends SolrCloudTestCase {
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 770742c4547..dedb4550dbb 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
@@ -1932,17 +1932,16 @@ public class OverseerTest extends SolrTestCaseJ4 {
}
private SolrCloudManager getCloudDataProvider(ZkStateReader zkStateReader) {
- var httpSolrClient =
+ var httpSolrClientBuilder =
new Http2SolrClient.Builder()
.withIdleTimeout(30000, TimeUnit.MILLISECONDS)
- .withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
- .build();
+ .withConnectionTimeout(15000, TimeUnit.MILLISECONDS);
var cloudSolrClient =
new CloudHttp2SolrClient.Builder(new
ZkClientClusterStateProvider(zkStateReader))
- .withHttpClient(httpSolrClient)
+ .withInternalClientBuilder(httpSolrClientBuilder)
.build();
solrClients.add(cloudSolrClient);
- solrClients.add(httpSolrClient);
+ solrClients.add(httpSolrClientBuilder.build());
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 0adb49471dc..68246001a40 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,7 +18,11 @@ package org.apache.solr.client.solrj;
import java.io.IOException;
-/** A lambda intended for invoking SolrClient operations */
+/**
+ * A lambda intended for invoking SolrClient operations
+ *
+ * @lucene.experimental
+ */
@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 ade1ebe433f..18aa318f8ba 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,9 +40,8 @@ import org.apache.solr.common.SolrException;
public class CloudHttp2SolrClient extends CloudSolrClient {
private final ClusterStateProvider stateProvider;
- private final LBHttp2SolrClient<Http2SolrClient> lbClient;
+ private final LBHttp2SolrClient<Http2SolrClient.Builder> 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
@@ -54,8 +53,8 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
*/
protected CloudHttp2SolrClient(Builder builder) {
super(builder.shardLeadersOnly, builder.parallelUpdates,
builder.directUpdatesToLeadersOnly);
- this.clientIsInternal = builder.httpClient == null;
- this.myClient = createOrGetHttpClientFromBuilder(builder);
+ var httpSolrClientBuilder = createOrGetHttpClientBuilder(builder);
+ this.myClient = httpSolrClientBuilder.build();
this.stateProvider = createClusterStateProvider(builder);
this.retryExpiryTimeNano = builder.retryExpiryTimeNano;
this.defaultCollection = builder.defaultCollection;
@@ -73,16 +72,14 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
// locks.
this.locks = objectList(builder.parallelCacheRefreshesLocks);
- this.lbClient = new
LBHttp2SolrClient.Builder<Http2SolrClient>(myClient).build();
+ this.lbClient = new
LBHttp2SolrClient.Builder<>(httpSolrClientBuilder).build();
}
- private Http2SolrClient createOrGetHttpClientFromBuilder(Builder builder) {
- if (builder.httpClient != null) {
- return builder.httpClient;
- } else if (builder.internalClientBuilder != null) {
- return builder.internalClientBuilder.build();
+ private Http2SolrClient.Builder createOrGetHttpClientBuilder(Builder
builder) {
+ if (builder.internalClientBuilder != null) {
+ return builder.internalClientBuilder;
} else {
- return new Http2SolrClient.Builder().build();
+ return new Http2SolrClient.Builder();
}
}
@@ -129,7 +126,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
private void closeMyClientIfNeeded() {
try {
- if (clientIsInternal && myClient != null) {
+ if (myClient != null) {
myClient.close();
}
} catch (Exception e) {
@@ -148,7 +145,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
}
@Override
- public LBHttp2SolrClient<Http2SolrClient> getLbClient() {
+ public LBHttp2SolrClient<Http2SolrClient.Builder> getLbClient() {
return lbClient;
}
@@ -171,7 +168,6 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
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;
@@ -404,23 +400,6 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
return this;
}
- /**
- * 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
@@ -430,10 +409,6 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
* @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 1fe2b91eb83..60a206dc8cc 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 @@ public class Http2SolrClient extends HttpSolrClientBase {
}
// note: do not manipulate httpClient below this point; it could be a
shared instance
- if (builder.listenerFactory != null) {
- this.listenerFactory.addAll(builder.listenerFactory);
+ if (builder.listenerFactories != null) {
+ this.listenerFactory.addAll(builder.listenerFactories);
}
updateDefaultMimeTypeForParser();
this.idleTimeoutMillis = builder.getIdleTimeoutMillis();
@@ -628,6 +628,7 @@ public class Http2SolrClient extends HttpSolrClientBase {
* @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)
@@ -958,7 +959,7 @@ public class Http2SolrClient extends HttpSolrClientBase {
protected Long keyStoreReloadIntervalSecs;
- private List<HttpListenerFactory> listenerFactory;
+ private List<HttpListenerFactory> listenerFactories = new ArrayList<>(0);
public Builder() {
super();
@@ -984,8 +985,27 @@ public class Http2SolrClient extends HttpSolrClientBase {
this.baseSolrUrl = baseSolrUrl;
}
- public Http2SolrClient.Builder
withListenerFactory(List<HttpListenerFactory> listenerFactory) {
- this.listenerFactory = listenerFactory;
+ /**
+ * 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);
return this;
}
@@ -1103,8 +1123,8 @@ public class Http2SolrClient extends HttpSolrClientBase {
if (this.urlParamNames == null) {
this.urlParamNames = http2SolrClient.urlParamNames;
}
- if (this.listenerFactory == null) {
- this.listenerFactory = new
ArrayList<>(http2SolrClient.listenerFactory);
+ if (this.listenerFactories == null) {
+ this.listenerFactories = 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 0bcecb2d89d..714c5b1f71d 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 class HttpJdkSolrClient extends HttpSolrClientBase {
public CompletableFuture<NamedList<Object>> requestAsync(
final SolrRequest<?> solrRequest, String collection) {
try {
- PreparedRequest pReq = prepareRequest(solrRequest, collection, null);
+ PreparedRequest pReq = prepareRequest(solrRequest, collection);
return httpClient
.sendAsync(pReq.reqb.build(),
HttpResponse.BodyHandlers.ofInputStream())
.thenApply(
@@ -164,10 +164,10 @@ public class HttpJdkSolrClient extends HttpSolrClientBase
{
}
}
- protected NamedList<Object> requestWithBaseUrl(
- String baseUrl, SolrRequest<?> solrRequest, String collection)
+ @Override
+ public NamedList<Object> request(SolrRequest<?> solrRequest, String
collection)
throws SolrServerException, IOException {
- PreparedRequest pReq = prepareRequest(solrRequest, collection, baseUrl);
+ PreparedRequest pReq = prepareRequest(solrRequest, collection);
HttpResponse<InputStream> response = null;
try {
response = httpClient.send(pReq.reqb.build(),
HttpResponse.BodyHandlers.ofInputStream());
@@ -199,25 +199,13 @@ public class HttpJdkSolrClient extends HttpSolrClientBase
{
}
}
- @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)
+ private PreparedRequest prepareRequest(SolrRequest<?> solrRequest, String
collection)
throws SolrServerException, IOException {
checkClosed();
if (ClientUtils.shouldApplyDefaultCollection(collection, solrRequest)) {
collection = defaultCollection;
}
- String url;
- if (overrideBaseUrl != null) {
- url = ClientUtils.buildRequestUrl(solrRequest, overrideBaseUrl,
collection);
- } else {
- url = getRequestUrl(solrRequest, collection);
- }
+ String 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 28039a574f9..1a796cfb9e7 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,22 +21,25 @@ import java.net.ConnectException;
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 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.
+ * 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.
*
* <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.
@@ -54,7 +57,7 @@ import org.slf4j.MDC;
* <blockquote>
*
* <pre>
- * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClient,
+ * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClientBuilder,
* new LBSolrClient.Endpoint("http://host1:8080/solr";), new
LBSolrClient.Endpoint("http://host2:8080/solr";))
* .build();
* </pre>
@@ -67,7 +70,7 @@ import org.slf4j.MDC;
* <blockquote>
*
* <pre>
- * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClient,
+ * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClientBuilder,
* new LBSolrClient.Endpoint("http://host1:8080/solr";, "coreA"),
* new LBSolrClient.Endpoint("http://host2:8080/solr";, "coreB"))
* .build();
@@ -92,35 +95,63 @@ import org.slf4j.MDC;
*
* @since solr 8.0
*/
-public class LBHttp2SolrClient<C extends HttpSolrClientBase> extends
LBSolrClient {
+public class LBHttp2SolrClient<B extends HttpSolrClientBuilderBase<?, ?>>
extends LBSolrClient {
- protected final C solrClient;
+ private final Map<String, HttpSolrClientBase> urlToClient;
+ private final Set<String> urlParamNames;
+
+ // must synchronize on this when building
+ private final HttpSolrClientBuilderBase<?, ?> solrClientBuilder;
- @SuppressWarnings("unchecked")
private LBHttp2SolrClient(Builder<?> builder) {
super(Arrays.asList(builder.solrEndpoints));
- this.solrClient = (C) builder.solrClient;
+ this.solrClientBuilder = builder.solrClientBuilder;
+
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 SolrClient getClient(Endpoint endpoint) {
- return solrClient;
+ protected HttpSolrClientBase getClient(final Endpoint endpoint) {
+ return urlToClient.computeIfAbsent(
+ endpoint.getBaseUrl(),
+ url -> {
+ synchronized (solrClientBuilder) {
+ solrClientBuilder.baseSolrUrl = url;
+ return solrClientBuilder.build();
+ }
+ });
}
@Override
public ResponseParser getParser() {
- return solrClient.getParser();
+ return urlToClient.isEmpty() ? null :
urlToClient.values().iterator().next().getParser();
}
@Override
public RequestWriter getRequestWriter() {
- return solrClient.getRequestWriter();
+ return urlToClient.isEmpty() ? null :
urlToClient.values().iterator().next().getRequestWriter();
}
public Set<String> getUrlParamNames() {
- return solrClient.getUrlParamNames();
+ return urlParamNames;
+ }
+
+ @Override
+ public void close() {
+ urlToClient.values().forEach(IOUtils::closeQuietly);
+ super.close();
}
/**
@@ -214,23 +245,18 @@ public class LBHttp2SolrClient<C extends
HttpSolrClientBase> extends LBSolrClien
RetryListener listener) {
String baseUrl = endpoint.toString();
rsp.server = baseUrl;
- 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);
- }
+ 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;
}
private void onSuccessfulRequest(
@@ -294,16 +320,28 @@ public class LBHttp2SolrClient<C extends
HttpSolrClientBase> extends LBSolrClien
}
}
- public static class Builder<C extends HttpSolrClientBase> {
+ public static class Builder<B extends HttpSolrClientBuilderBase<?, ?>> {
+
+ 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;
- public Builder(C solrClient, Endpoint... endpoints) {
- this.solrClient = solrClient;
+ /**
+ * 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;
this.solrEndpoints = endpoints;
}
@@ -313,7 +351,7 @@ public class LBHttp2SolrClient<C extends
HttpSolrClientBase> extends LBSolrClien
*
* @param aliveCheckInterval how often to ping for aliveness
*/
- public Builder<C> setAliveCheckInterval(int aliveCheckInterval, TimeUnit
unit) {
+ public Builder<B> setAliveCheckInterval(int aliveCheckInterval, TimeUnit
unit) {
if (aliveCheckInterval <= 0) {
throw new IllegalArgumentException(
"Alive check interval must be " + "positive, specified value = " +
aliveCheckInterval);
@@ -323,13 +361,13 @@ public class LBHttp2SolrClient<C extends
HttpSolrClientBase> extends LBSolrClien
}
/** Sets a default for core or collection based requests. */
- public Builder<C> withDefaultCollection(String defaultCoreOrCollection) {
+ public Builder<B> withDefaultCollection(String defaultCoreOrCollection) {
this.defaultCollection = defaultCoreOrCollection;
return this;
}
- public LBHttp2SolrClient<C> build() {
- return new LBHttp2SolrClient<C>(this);
+ public LBHttp2SolrClient<B> build() {
+ return new LBHttp2SolrClient<B>(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 85fadde4ac7..132570b4643 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,25 +495,7 @@ public abstract class LBSolrClient extends SolrClient {
private NamedList<Object> doRequest(Endpoint endpoint, SolrRequest<?>
solrRequest)
throws SolrServerException, IOException {
final var solrClient = getClient(endpoint);
- 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);
+ return solrClient.request(solrRequest, endpoint.getCore());
}
protected Exception doRequest(
@@ -622,12 +604,7 @@ public abstract class LBSolrClient extends SolrClient {
// First the one on the endpoint, then the default collection
final String effectiveCollection =
Objects.requireNonNullElse(zombieEndpoint.getCore(),
getDefaultCollection());
- final var responseRaw =
- doRequest(
- getClient(zombieEndpoint),
- zombieEndpoint.getBaseUrl(),
- effectiveCollection,
- queryRequest);
+ final var responseRaw = getClient(zombieEndpoint).request(queryRequest,
effectiveCollection);
QueryResponse resp = new QueryResponse();
resp.setResponse(responseRaw);
@@ -729,7 +706,7 @@ public abstract class LBSolrClient extends SolrClient {
// Choose the endpoint's core/collection over any specified by the user
final var effectiveCollection =
endpoint.getCore() == null ? collection : endpoint.getCore();
- return doRequest(getClient(endpoint), endpoint.getBaseUrl(),
effectiveCollection, request);
+ return getClient(endpoint).request(request, effectiveCollection);
} catch (SolrException e) {
// Server is alive but the request was malformed or invalid
throw e;
@@ -773,8 +750,7 @@ public abstract class LBSolrClient extends SolrClient {
++numServersTried;
final String effectiveCollection =
endpoint.getCore() == null ? collection : endpoint.getCore();
- NamedList<Object> rsp =
- doRequest(getClient(endpoint), endpoint.getBaseUrl(),
effectiveCollection, request);
+ NamedList<Object> rsp = getClient(endpoint).request(request,
effectiveCollection);
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 0846dfefc6c..46b6883f755 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
@@ -120,26 +120,6 @@ public class CloudHttp2SolrClientBuilderTest extends
SolrCloudTestCase {
}
}
- @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);
@@ -159,20 +139,6 @@ public class CloudHttp2SolrClientBuilderTest extends
SolrCloudTestCase {
verify(http2Client, times(1)).close();
}
- @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 =
@@ -195,29 +161,19 @@ public class CloudHttp2SolrClientBuilderTest extends
SolrCloudTestCase {
public void testHttpClientPreservedInHttp2ClusterStateProvider() throws
IOException {
List<String> solrUrls =
List.of(cluster.getJettySolrRunner(0).getBaseUrl().toString());
- // No httpClient - No internalClientBuilder
- testHttpClientConsistency(solrUrls, null, null);
-
- // httpClient - No internalClientBuilder
- try (Http2SolrClient httpClient = new Http2SolrClient.Builder().build()) {
- testHttpClientConsistency(solrUrls, httpClient, null);
- }
+ // without internalClientBuilder
+ testHttpClientConsistency(solrUrls, null);
- // No httpClient - internalClientBuilder
+ // with internalClientBuilder
Http2SolrClient.Builder internalClientBuilder = new
Http2SolrClient.Builder();
- testHttpClientConsistency(solrUrls, null, internalClientBuilder);
+ testHttpClientConsistency(solrUrls, internalClientBuilder);
}
private void testHttpClientConsistency(
- List<String> solrUrls,
- Http2SolrClient httpClient,
- Http2SolrClient.Builder internalClientBuilder)
- throws IOException {
+ List<String> solrUrls, Http2SolrClient.Builder internalClientBuilder)
throws IOException {
CloudHttp2SolrClient.Builder clientBuilder = new
CloudHttp2SolrClient.Builder(solrUrls);
- if (httpClient != null) {
- clientBuilder.withHttpClient(httpClient);
- } else if (internalClientBuilder != null) {
+ 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 e6a47680bc8..11fcad658ca 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,7 +35,6 @@ import org.apache.solr.client.solrj.SolrClient;
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;
@@ -152,26 +151,6 @@ public class HttpJdkSolrClientTest extends
HttpSolrClientTestBase {
}
}
- @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 3f24984f90e..e9e63683dbe 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,7 +17,6 @@
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;
@@ -118,30 +117,28 @@ public class LBHttp2SolrClientIntegrationTest extends
SolrTestCaseJ4 {
private LBClientHolder client(LBSolrClient.Endpoint... baseSolrEndpoints) {
if (random().nextBoolean()) {
- var delegateClient =
+ var delegateClientBuilder =
new Http2SolrClient.Builder()
.withConnectionTimeout(1000, TimeUnit.MILLISECONDS)
- .withIdleTimeout(2000, TimeUnit.MILLISECONDS)
- .build();
+ .withIdleTimeout(2000, TimeUnit.MILLISECONDS);
var lbClient =
- new LBHttp2SolrClient.Builder<>(delegateClient, baseSolrEndpoints)
+ new LBHttp2SolrClient.Builder<>(delegateClientBuilder,
baseSolrEndpoints)
.withDefaultCollection(solr[0].getDefaultCollection())
.setAliveCheckInterval(500, TimeUnit.MILLISECONDS)
.build();
- return new LBClientHolder(lbClient, delegateClient);
+ return new LBClientHolder(lbClient, delegateClientBuilder);
} else {
- var delegateClient =
+ var delegateClientBuilder =
new HttpJdkSolrClient.Builder()
.withConnectionTimeout(1000, TimeUnit.MILLISECONDS)
.withIdleTimeout(2000, TimeUnit.MILLISECONDS)
- .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT)
- .build();
+ .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT);
var lbClient =
- new LBHttp2SolrClient.Builder<>(delegateClient, baseSolrEndpoints)
+ new LBHttp2SolrClient.Builder<>(delegateClientBuilder,
baseSolrEndpoints)
.withDefaultCollection(solr[0].getDefaultCollection())
.setAliveCheckInterval(500, TimeUnit.MILLISECONDS)
.build();
- return new LBClientHolder(lbClient, delegateClient);
+ return new LBClientHolder(lbClient, delegateClientBuilder);
}
}
@@ -317,9 +314,9 @@ public class LBHttp2SolrClientIntegrationTest extends
SolrTestCaseJ4 {
private static class LBClientHolder implements AutoCloseable {
final LBHttp2SolrClient<?> lbClient;
- final HttpSolrClientBase delegate;
+ final HttpSolrClientBuilderBase<?, ?> delegate;
- LBClientHolder(LBHttp2SolrClient<?> lbClient, HttpSolrClientBase delegate)
{
+ LBClientHolder(LBHttp2SolrClient<?> lbClient, HttpSolrClientBuilderBase<?,
?> delegate) {
this.lbClient = lbClient;
this.delegate = delegate;
}
@@ -327,11 +324,6 @@ public class LBHttp2SolrClientIntegrationTest extends
SolrTestCaseJ4 {
@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 9d2019309b0..30cee0804b5 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,6 +18,8 @@ package org.apache.solr.client.solrj.impl;
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;
@@ -28,7 +30,6 @@ import java.util.concurrent.ExecutionException;
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;
@@ -51,11 +52,12 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
Set<String> urlParamNames = new HashSet<>(2);
urlParamNames.add("param1");
- try (Http2SolrClient http2SolrClient =
- new
Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames).build();
- LBHttp2SolrClient<Http2SolrClient> testClient =
- new LBHttp2SolrClient.Builder<>(http2SolrClient, new
LBSolrClient.Endpoint(url))
- .build()) {
+ 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()) {
assertArrayEquals(
"Wrong urlParamNames found in lb client.",
@@ -64,7 +66,7 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
assertArrayEquals(
"Wrong urlParamNames found in base client.",
urlParamNames.toArray(),
- http2SolrClient.getUrlParamNames().toArray());
+ testClient.getClient(endpoint).getUrlParamNames().toArray());
}
}
@@ -74,12 +76,11 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
LBSolrClient.Endpoint ep2 = new
LBSolrClient.Endpoint("http://endpoint.two";);
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);
- 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()) {
+ var httpSolrClientBuilder =
+ new MockHttpSolrClientBuilder().withConnectionTimeout(10,
TimeUnit.SECONDS);
+
+ try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
+ new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1,
ep2).build()) {
String lastEndpoint = null;
for (int i = 0; i < 10; i++) {
@@ -103,15 +104,14 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
LBSolrClient.Endpoint ep2 = new
LBSolrClient.Endpoint("http://endpoint.two";);
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);
- 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()) {
+ var httpSolrClientBuilder =
+ new MockHttpSolrClientBuilder().withConnectionTimeout(10,
TimeUnit.SECONDS);
+
+ try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
+ new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1,
ep2).build()) {
- client.basePathToFail = ep1.getBaseUrl();
- String basePathToSucceed = ep2.getBaseUrl();
+ setEndpointToFail(testClient, ep1);
+ setEndpointToSucceed(testClient, ep2);
String qValue = "First time";
for (int i = 0; i < 5; i++) {
@@ -121,13 +121,13 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
LBSolrClient.Rsp response = testClient.request(req);
assertEquals(
"The healthy node 'endpoint two' should have served the request: "
+ i,
- basePathToSucceed,
+ ep2.getBaseUrl(),
response.server);
checkSynchonousResponseContent(response, qValue);
}
- client.basePathToFail = ep2.getBaseUrl();
- basePathToSucceed = ep1.getBaseUrl();
+ setEndpointToFail(testClient, ep2);
+ setEndpointToSucceed(testClient, ep1);
qValue = "Second time";
for (int i = 0; i < 5; i++) {
@@ -137,21 +137,13 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
LBSolrClient.Rsp response = testClient.request(req);
assertEquals(
"The healthy node 'endpoint one' should have served the request: "
+ i,
- basePathToSucceed,
+ ep1.getBaseUrl(),
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() {
@@ -162,28 +154,35 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
LBSolrClient.Endpoint ep2 = new
LBSolrClient.Endpoint("http://endpoint.two";);
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);
- 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()) {
+ var httpSolrClientBuilder =
+ new MockHttpSolrClientBuilder().withConnectionTimeout(10,
TimeUnit.SECONDS);
+
+ try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
+ new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, 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
- String basePathToSucceed;
+ LBSolrClient.Endpoint endpointToSucceed;
+ LBSolrClient.Endpoint endpointToFail;
if (j == 0) {
- client.basePathToFail = ep1.getBaseUrl();
- basePathToSucceed = ep2.getBaseUrl();
+ setEndpointToFail(testClient, ep1);
+ setEndpointToSucceed(testClient, ep2);
+ endpointToSucceed = ep2;
+ endpointToFail = ep1;
} else {
- client.basePathToFail = ep2.getBaseUrl();
- basePathToSucceed = ep1.getBaseUrl();
+ setEndpointToFail(testClient, ep2);
+ setEndpointToSucceed(testClient, ep1);
+ endpointToSucceed = ep1;
+ endpointToFail = ep2;
}
+ 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 to see if it behaves the same every time.
+ // i: we'll try 10 times. It should behave the same with iter 2-10.
.
QueryRequest queryRequest = new QueryRequest(new
MapSolrParams(Map.of("q", "" + i)));
LBSolrClient.Req req = new LBSolrClient.Req(queryRequest,
endpointList);
@@ -196,26 +195,26 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
} catch (TimeoutException | ExecutionException e) {
fail(iterMessage + " Response ended in failure: " + e);
}
+
if (i == 0) {
- // When j=0, "endpoint one" fails.
- // The first time around (i) it tries the first, then the second.
- //
- // With j=0 and i>0, it only tries "endpoint two".
+ // When i=0, it must try both endpoints to find success:
//
- // 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);
+ // 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());
} else {
- // The first endpoint does not give the exception, it doesn't
retry.
- assertEquals(iterMessage, 1, client.lastBasePaths.size());
+ // 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());
}
- String successBasePath = client.lastBasePaths.remove(0);
- assertEquals(iterMessage, basePathToSucceed, successBasePath);
+ successEndpointLastBasePaths.clear();
+ failEndpointLastBasePaths.clear();
}
}
}
@@ -227,11 +226,11 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
LBSolrClient.Endpoint ep2 = new
LBSolrClient.Endpoint("http://endpoint.two";);
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);
- 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()) {
+ var httpSolrClientBuilder =
+ new MockHttpSolrClientBuilder().withConnectionTimeout(10,
TimeUnit.SECONDS);
+
+ try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
+ new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1,
ep2).build()) {
int limit = 10; // For simplicity use an even limit
List<CompletableFuture<LBSolrClient.Rsp>> responses = new ArrayList<>();
@@ -243,23 +242,17 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
}
QueryRequest[] queryRequests = new QueryRequest[limit];
- int numEndpointOne = 0;
- int numEndpointTwo = 0;
+ List<SolrRequest<?>> lastSolrRequests = lastSolrRequests(testClient,
ep1, ep2);
+ assertEquals(limit, lastSolrRequests.size());
+
for (int i = 0; i < limit; i++) {
- SolrRequest<?> lastSolrReq = client.lastSolrRequests.get(i);
+ SolrRequest<?> lastSolrReq = 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();
@@ -278,15 +271,55 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
// 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!
- assertEquals(limit, numEndpointOne);
- assertEquals(0, numEndpointTwo);
+ List<String> ep1BasePaths = basePathsForEndpoint(testClient, ep1);
+ List<String> ep2BasePaths = basePathsForEndpoint(testClient, ep2);
+ assertEquals(limit, basePathsForEndpoint(testClient, ep1).size());
+ assertEquals(0, basePathsForEndpoint(testClient, ep2).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;
+ }
- assertEquals(limit, client.lastSolrRequests.size());
- assertEquals(limit, client.lastCollections.size());
+ 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 Http2SolrClient {
+ public static class MockHttpSolrClient extends HttpSolrClientBase {
public List<SolrRequest<?>> lastSolrRequests = new ArrayList<>();
@@ -294,15 +327,13 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
public List<String> lastCollections = new ArrayList<>();
- public String basePathToFail = null;
+ public boolean allRequestsShallFail;
public String tmpBaseUrl = null;
- protected MockHttpSolrClient(String serverBaseUrl, Builder builder) {
+ public boolean closeCalled;
- // TODO: Consider creating an interface for Http*SolrClient
- // so mocks can Implement, not Extend, and not actually need to
- // build an (unused) client
+ protected MockHttpSolrClient(String serverBaseUrl,
MockHttpSolrClientBuilder builder) {
super(serverBaseUrl, builder);
}
@@ -312,25 +343,12 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
lastSolrRequests.add(request);
lastBasePaths.add(tmpBaseUrl);
lastCollections.add(collection);
- if (tmpBaseUrl.equals(basePathToFail)) {
+ if (allRequestsShallFail) {
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) {
@@ -338,7 +356,7 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
lastSolrRequests.add(solrRequest);
lastBasePaths.add(tmpBaseUrl);
lastCollections.add(collection);
- if (tmpBaseUrl != null && tmpBaseUrl.equals(basePathToFail)) {
+ if (allRequestsShallFail) {
cf.completeExceptionally(
new SolrException(SolrException.ErrorCode.SERVER_ERROR, "We should
retry this."));
} else {
@@ -351,5 +369,32 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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
+ }
}
}
