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 704f2fe3f72 Revert "SOLR-17541: LBSolrClient implementations should
agree on 'getClient()' semantics (#2899)"
704f2fe3f72 is described below
commit 704f2fe3f72fd2402d14d16c2f9d6e866869723d
Author: jdyer1 <[email protected]>
AuthorDate: Mon Dec 23 17:41:16 2024 -0600
Revert "SOLR-17541: LBSolrClient implementations should agree on
'getClient()' semantics (#2899)"
This reverts commit d3e57aad8259f4b9c574fbff5cb5f6d112bef85a.
---
solr/CHANGES.txt | 14 +-
.../java/org/apache/solr/cloud/ZkController.java | 11 +-
.../apache/solr/core/HttpSolrClientProvider.java | 13 +-
.../solr/handler/component/HttpShardHandler.java | 2 +-
.../handler/component/HttpShardHandlerFactory.java | 15 +-
.../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 | 36 +--
.../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 | 20 ++
.../impl/LBHttp2SolrClientIntegrationTest.java | 28 ++-
.../client/solrj/impl/LBHttp2SolrClientTest.java | 247 +++++++++------------
19 files changed, 348 insertions(+), 344 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a135ecaa539..a844220f2fc 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -34,10 +34,6 @@ Improvements
* SOLR-17516: `LBHttp2SolrClient` is now generic, adding support for
`HttpJdkSolrClient`. (James Dyer)
-* 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.
@@ -106,11 +102,6 @@ Deprecation Removals
* SOLR-17540: Removed the Hadoop Auth module, and thus Kerberos authentication
and other exotic options. (Eric Pugh)
-* SOLR-17541: Removed `CloudHttp2SolrClient.Builder#withHttpClient` in favor
of `CloudHttp2SolrClient.Builder#withInternalClientBuilder`.
- The constructor on `LBHttp2SolrClient.Builder` that took an instance of
`HttpSolrClientBase` is updated to instead take an instance of
- `HttpSolrClientBuilderBase`. Renamed
`LBHttp2SolrClient.Builder#withListenerFactory` to
`LBHttp2SolrClient.Builder#withListenerFactories`
- (James Dyer)
-
Dependency Upgrades
---------------------
(No changes)
@@ -159,10 +150,7 @@ New Features
Improvements
---------------------
-* SOLR-17541: Deprecate `CloudHttp2SolrClient.Builder#withHttpClient` in favor
of
- `CloudHttp2SolrClient.Builder#withInternalClientBuilder`.
- Deprecate `LBHttp2SolrClient.Builder#withListenerFactory` in favor of
- `LBHttp2SolrClient.Builder#withListenerFactories` (James Dyer)
+(No changes)
Optimizations
---------------------
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 5a890121a43..7dd30a14d24 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -197,6 +197,9 @@ public class ZkController implements Closeable {
public final ZkStateReader zkStateReader;
private SolrCloudManager cloudManager;
+ // only for internal usage
+ private Http2SolrClient http2SolrClient;
+
private CloudHttp2SolrClient cloudSolrClient;
private final String zkServerAddress; // example: 127.0.0.1:54062/solr
@@ -751,6 +754,7 @@ public class ZkController implements Closeable {
sysPropsCacher.close();
customThreadPool.execute(() -> IOUtils.closeQuietly(cloudManager));
customThreadPool.execute(() -> IOUtils.closeQuietly(cloudSolrClient));
+ customThreadPool.execute(() -> IOUtils.closeQuietly(http2SolrClient));
try {
try {
@@ -846,14 +850,15 @@ public class ZkController implements Closeable {
if (cloudManager != null) {
return cloudManager;
}
- var httpSolrClientBuilder =
+ http2SolrClient =
new Http2SolrClient.Builder()
.withHttpClient(cc.getDefaultHttpSolrClient())
.withIdleTimeout(30000, TimeUnit.MILLISECONDS)
- .withConnectionTimeout(15000, TimeUnit.MILLISECONDS);
+ .withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
+ .build();
cloudSolrClient =
new CloudHttp2SolrClient.Builder(new
ZkClientClusterStateProvider(zkStateReader))
- .withInternalClientBuilder(httpSolrClientBuilder)
+ .withHttpClient(http2SolrClient)
.build();
cloudManager = new SolrClientCloudManager(cloudSolrClient,
cc.getObjectCache());
cloudManager.getClusterStateProvider().connect();
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 e9631b26d1f..2bf25a896f6 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 @@ 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);
- 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 @@ final class HttpSolrClientProvider implements AutoCloseable {
}
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 320a5fe70d7..7592eed86fc 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
@@ -114,7 +114,7 @@ public class HttpShardHandler extends ShardHandler {
protected AtomicInteger pending;
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 1437dee63ea..ac7dc0cf8e0 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
@@ -83,9 +83,8 @@ public class HttpShardHandlerFactory extends
ShardHandlerFactory
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;
@@ -306,16 +305,16 @@ public class HttpShardHandlerFactory extends
ShardHandlerFactory
sb);
int soTimeout =
getParameter(args, HttpClientUtil.PROP_SO_TIMEOUT,
HttpClientUtil.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));
@@ -325,7 +324,7 @@ public class HttpShardHandlerFactory extends
ShardHandlerFactory
@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 206fb3d0886..b43d5f22190 100644
--- a/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
@@ -34,9 +34,4 @@ public interface HttpClientBuilderPlugin {
public SolrHttpClientBuilder getHttpClientBuilder(SolrHttpClientBuilder
builder);
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 93ecdcc9d68..b1f6e6b6eed 100644
--- a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
@@ -376,11 +376,6 @@ 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";
@@ -436,12 +431,7 @@ public class PKIAuthenticationPlugin extends
AuthenticationPlugin
(String) request.getAttributes().get(CACHED_REQUEST_USER_KEY));
}
};
- if (client != null) {
- client.addListenerFactory(() -> listener);
- }
- if (builder != null) {
- builder.addListenerFactory(() -> listener);
- }
+ client.addListenerFactory(() -> listener);
}
@Override
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 9eb3e783a2d..758ae9ccd51 100644
--- a/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
+++ b/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
@@ -76,7 +76,6 @@ public class PostToolTest extends SolrCloudTestCase {
withBasicAuth(CollectionAdminRequest.createCollection(collection, "conf1",
1, 1, 0, 0))
.processAndWait(cluster.getSolrClient(), 10);
- waitForState("creating", collection, activeClusterShape(1, 1));
File jsonDoc = File.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 e611902898f..bad8d58d021 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
@@ -1945,16 +1945,17 @@ public class OverseerTest extends SolrTestCaseJ4 {
}
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 68246001a40..0adb49471dc 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 @@ package org.apache.solr.client.solrj;
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 18aa318f8ba..ade1ebe433f 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 @@ import org.apache.solr.common.SolrException;
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 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
*/
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 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
// 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 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
private void closeMyClientIfNeeded() {
try {
- if (myClient != null) {
+ if (clientIsInternal && myClient != null) {
myClient.close();
}
} catch (Exception e) {
@@ -145,7 +148,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
}
@Override
- public LBHttp2SolrClient<Http2SolrClient.Builder> getLbClient() {
+ public LBHttp2SolrClient<Http2SolrClient> getLbClient() {
return lbClient;
}
@@ -168,6 +171,7 @@ 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;
@@ -400,6 +404,23 @@ 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
@@ -409,6 +430,10 @@ 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 fddc4e2daa6..fb2eb1a123f 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
@@ -140,7 +140,9 @@ public class Http2SolrClient extends HttpSolrClientBase {
this.httpClient = createHttpClient(builder);
this.closeClient = true;
}
- this.listenerFactory.addAll(builder.listenerFactories);
+ if (builder.listenerFactory != null) {
+ this.listenerFactory.addAll(builder.listenerFactory);
+ }
updateDefaultMimeTypeForParser();
this.httpClient.setFollowRedirects(Boolean.TRUE.equals(builder.followRedirects));
@@ -569,7 +571,6 @@ 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)
@@ -905,7 +906,7 @@ public class Http2SolrClient extends HttpSolrClientBase {
protected Long keyStoreReloadIntervalSecs;
- private List<HttpListenerFactory> listenerFactories = new ArrayList<>(0);
+ private List<HttpListenerFactory> listenerFactory;
public Builder() {
super();
@@ -931,27 +932,8 @@ public class Http2SolrClient extends HttpSolrClientBase {
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;
}
@@ -1127,9 +1109,9 @@ public class Http2SolrClient extends HttpSolrClientBase {
if (this.urlParamNames == null) {
this.urlParamNames = http2SolrClient.urlParamNames;
}
- if (this.listenerFactories.isEmpty()) {
- this.listenerFactories.clear();
- http2SolrClient.listenerFactory.forEach(this.listenerFactories::add);
+ if (this.listenerFactory == null) {
+ this.listenerFactory = new ArrayList<HttpListenerFactory>();
+ http2SolrClient.listenerFactory.forEach(this.listenerFactory::add);
}
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 c26d13606b6..1127b3fd1a1 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
@@ -138,7 +138,7 @@ public class HttpJdkSolrClient extends HttpSolrClientBase {
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(
@@ -157,10 +157,10 @@ public class HttpJdkSolrClient extends HttpSolrClientBase
{
}
}
- @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());
@@ -192,13 +192,25 @@ public class HttpJdkSolrClient extends HttpSolrClientBase
{
}
}
- 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 4c0d46b13db..2c926a26261 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
@@ -23,25 +23,22 @@ 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.SolrServerException;
import org.apache.solr.client.solrj.request.IsUpdateRequest;
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.
@@ -59,7 +56,7 @@ import org.slf4j.MDC;
* <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>
@@ -72,7 +69,7 @@ import org.slf4j.MDC;
* <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();
@@ -97,63 +94,35 @@ import org.slf4j.MDC;
*
* @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;
-
- // must synchronize on this when building
- private final HttpSolrClientBuilderBase<?, ?> solrClientBuilder;
+ protected final C solrClient;
+ @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();
}
/**
@@ -241,18 +210,23 @@ public class LBHttp2SolrClient<B extends
HttpSolrClientBuilderBase<?, ?>> extend
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(
@@ -316,28 +290,16 @@ public class LBHttp2SolrClient<B extends
HttpSolrClientBuilderBase<?, ?>> extend
}
}
- public static class Builder<B extends HttpSolrClientBuilderBase<?, ?>> {
-
- private final B solrClientBuilder;
+ public static class Builder<C extends HttpSolrClientBase> {
+ 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;
}
@@ -347,7 +309,7 @@ public class LBHttp2SolrClient<B extends
HttpSolrClientBuilderBase<?, ?>> extend
*
* @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);
@@ -357,13 +319,13 @@ public class LBHttp2SolrClient<B extends
HttpSolrClientBuilderBase<?, ?>> extend
}
/** 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 31c75662b48..64201b03c13 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
@@ -497,7 +497,25 @@ public abstract class LBSolrClient extends SolrClient {
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(
@@ -607,7 +625,12 @@ 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 = getClient(zombieEndpoint).request(queryRequest,
effectiveCollection);
+ final var responseRaw =
+ doRequest(
+ getClient(zombieEndpoint),
+ zombieEndpoint.getBaseUrl(),
+ effectiveCollection,
+ queryRequest);
QueryResponse resp = new QueryResponse();
resp.setResponse(responseRaw);
@@ -711,7 +734,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 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;
@@ -752,7 +775,8 @@ public abstract class LBSolrClient extends SolrClient {
++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);
// remove from zombie list *before* adding to the alive list to avoid
a race that could lose
// a server
zombieServers.remove(endpoint.getUrl());
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 46b6883f755..0846dfefc6c 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,6 +120,26 @@ 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);
@@ -139,6 +159,20 @@ 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 =
@@ -161,19 +195,29 @@ public class CloudHttp2SolrClientBuilderTest extends
SolrCloudTestCase {
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);
+
+ // httpClient - No internalClientBuilder
+ try (Http2SolrClient httpClient = new Http2SolrClient.Builder().build()) {
+ testHttpClientConsistency(solrUrls, httpClient, null);
+ }
- // with internalClientBuilder
+ // 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 1dbfc0e998b..b3980ad44bc 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
@@ -34,6 +34,7 @@ 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.request.RequestWriter;
import org.apache.solr.client.solrj.response.SolrPingResponse;
import org.apache.solr.common.params.CommonParams;
@@ -153,6 +154,25 @@ 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 BinaryResponseParser());
+ try (HttpJdkSolrClient client = b.build()) {
+ client.requestWithBaseUrl(intendedUrl, new QueryRequest(q,
SolrRequest.METHOD.GET), null);
+ assertEquals(
+ client.getParser().getVersion(),
DebugServlet.parameters.get(CommonParams.VERSION)[0]);
+ }
+ }
+
@Test
public void testGetById() throws Exception {
DebugServlet.clear();
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 9c2f79ff0a5..61504a052b8 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
@@ -18,6 +18,7 @@ package org.apache.solr.client.solrj.impl;
import java.io.File;
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 @@ public class LBHttp2SolrClientIntegrationTest extends
SolrTestCaseJ4 {
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 @@ public class LBHttp2SolrClientIntegrationTest extends
SolrTestCaseJ4 {
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 @@ 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 30cee0804b5..9d2019309b0 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 @@ 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;
@@ -30,6 +28,7 @@ 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;
@@ -52,12 +51,11 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
assertArrayEquals(
"Wrong urlParamNames found in base client.",
urlParamNames.toArray(),
- testClient.getClient(endpoint).getUrlParamNames().toArray());
+ http2SolrClient.getUrlParamNames().toArray());
}
}
@@ -76,11 +74,12 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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()) {
- 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 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
} 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 and i>0, it only tries "endpoint two".
//
- // 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());
+ // 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 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
}
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 @@ 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!
- 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, numEndpointOne);
+ assertEquals(0, numEndpointTwo);
- 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);
+ assertEquals(limit, client.lastSolrRequests.size());
+ assertEquals(limit, client.lastCollections.size());
}
}
- public static class MockHttpSolrClient extends HttpSolrClientBase {
+ public static class MockHttpSolrClient extends Http2SolrClient {
public List<SolrRequest<?>> lastSolrRequests = new ArrayList<>();
@@ -327,13 +294,15 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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,12 +312,25 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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) {
@@ -356,7 +338,7 @@ public class LBHttp2SolrClientTest extends SolrTestCase {
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 @@ 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
- }
}
}
