This is an automated email from the ASF dual-hosted git repository.
sanjaydutt pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 34780b2d42a SOLR-17538: CloudHttp2SolrClient needs a custom
ClusterStateProvider option (#2832)
34780b2d42a is described below
commit 34780b2d42aa870a620a544c632a2dd241f81411
Author: Sanjay Dutt <[email protected]>
AuthorDate: Tue Nov 5 20:14:24 2024 +0530
SOLR-17538: CloudHttp2SolrClient needs a custom ClusterStateProvider option
(#2832)
* SOLR-17538: CloudHttp2SolrClient needs a custom ClusterStateProvider
option
---------
Co-authored-by: David Smiley <[email protected]>
(cherry picked from commit 783d7805ea7ef6defcb87cdec55a662935a938fb)
---
.../java/org/apache/solr/cloud/ZkController.java | 3 +-
.../test/org/apache/solr/cloud/OverseerTest.java | 8 ++---
.../client/solrj/impl/CloudHttp2SolrClient.java | 35 +++++++++++++++++-----
.../solr/client/solrj/impl/CloudSolrClient.java | 5 ++++
.../impl/CloudHttp2SolrClientBuilderTest.java | 24 ++++++++++-----
5 files changed, 54 insertions(+), 21 deletions(-)
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 da54f8d170e..f7b61765fcd 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -63,6 +63,7 @@ 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;
+import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
import org.apache.solr.client.solrj.request.CoreAdminRequest.WaitForState;
import org.apache.solr.cloud.overseer.ClusterStateMutator;
import org.apache.solr.cloud.overseer.OverseerAction;
@@ -881,7 +882,7 @@ public class ZkController implements Closeable {
.withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
.build();
cloudSolrClient =
- new CloudHttp2SolrClient.Builder(List.of(getZkServerAddress()),
Optional.empty())
+ new CloudHttp2SolrClient.Builder(new
ZkClientClusterStateProvider(zkStateReader))
.withHttpClient(http2SolrClient)
.build();
cloudManager = new SolrClientCloudManager(cloudSolrClient,
cc.getObjectCache());
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 b11801ed3ad..3e67ed8f14f 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
@@ -41,7 +41,6 @@ import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
-import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
@@ -55,6 +54,7 @@ import org.apache.solr.client.solrj.cloud.SolrCloudManager;
import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient;
import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.client.solrj.impl.SolrClientCloudManager;
+import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
import org.apache.solr.cloud.overseer.NodeMutator;
import org.apache.solr.cloud.overseer.OverseerAction;
import org.apache.solr.cloud.overseer.ZkWriteCommand;
@@ -1944,18 +1944,18 @@ public class OverseerTest extends SolrTestCaseJ4 {
when(zkController.getLeaderProps(anyString(), anyString(),
anyInt())).thenCallRealMethod();
when(zkController.getLeaderProps(anyString(), anyString(), anyInt(),
anyBoolean()))
.thenCallRealMethod();
-
doReturn(getCloudDataProvider(zkAddress)).when(zkController).getSolrCloudManager();
+
doReturn(getCloudDataProvider(reader)).when(zkController).getSolrCloudManager();
return zkController;
}
- private SolrCloudManager getCloudDataProvider(String zkAddress) {
+ private SolrCloudManager getCloudDataProvider(ZkStateReader zkStateReader) {
var httpSolrClient =
new Http2SolrClient.Builder()
.withIdleTimeout(30000, TimeUnit.MILLISECONDS)
.withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
.build();
var cloudSolrClient =
- new CloudHttp2SolrClient.Builder(Collections.singletonList(zkAddress),
Optional.empty())
+ new CloudHttp2SolrClient.Builder(new
ZkClientClusterStateProvider(zkStateReader))
.withHttpClient(httpSolrClient)
.build();
solrClients.add(cloudSolrClient);
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 266b90c223c..670bd2963ff 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
@@ -56,10 +56,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
super(builder.shardLeadersOnly, builder.parallelUpdates,
builder.directUpdatesToLeadersOnly);
this.clientIsInternal = builder.httpClient == null;
this.myClient = createOrGetHttpClientFromBuilder(builder);
- this.stateProvider =
- builder.zkHosts.isEmpty()
- ? createHttp2ClusterStateProvider(builder.solrUrls, myClient)
- : createZkClusterStateProvider(builder);
+ this.stateProvider = createClusterStateProvider(builder);
this.retryExpiryTimeNano = builder.retryExpiryTimeNano;
this.defaultCollection = builder.defaultCollection;
if (builder.requestWriter != null) {
@@ -89,6 +86,16 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
}
}
+ private ClusterStateProvider createClusterStateProvider(Builder builder) {
+ if (builder.stateProvider != null) {
+ return builder.stateProvider;
+ } else if (builder.zkHosts.isEmpty()) {
+ return createHttp2ClusterStateProvider(builder.solrUrls, this.myClient);
+ } else {
+ return createZkClusterStateProvider(builder);
+ }
+ }
+
private ClusterStateProvider createZkClusterStateProvider(Builder builder) {
try {
ClusterStateProvider stateProvider =
@@ -233,6 +240,11 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
if (zkChroot.isPresent()) this.zkChroot = zkChroot.get();
}
+ /** for an expert use-case */
+ public Builder(ClusterStateProvider stateProvider) {
+ this.stateProvider = stateProvider;
+ }
+
/** Whether to use the default ZK ACLs when building a ZK Client. */
public Builder canUseZkACLs(boolean canUseZkACLs) {
this.canUseZkACLs = canUseZkACLs;
@@ -451,12 +463,19 @@ public class CloudHttp2SolrClient extends CloudSolrClient
{
/** Create a {@link CloudHttp2SolrClient} based on the provided
configuration. */
public CloudHttp2SolrClient build() {
- if (!zkHosts.isEmpty() && !solrUrls.isEmpty()) {
+ int providedOptions = 0;
+ if (!zkHosts.isEmpty()) providedOptions++;
+ if (!solrUrls.isEmpty()) providedOptions++;
+ if (stateProvider != null) providedOptions++;
+
+ if (providedOptions > 1) {
throw new IllegalArgumentException(
- "Both zkHost(s) & solrUrl(s) have been specified. Only specify
one.");
- } else if (zkHosts.isEmpty() && solrUrls.isEmpty()) {
- throw new IllegalArgumentException("Both zkHosts and solrUrl cannot be
null.");
+ "Only one of zkHost(s), solrUrl(s), or stateProvider should be
specified.");
+ } else if (providedOptions == 0) {
+ throw new IllegalArgumentException(
+ "One of zkHosts, solrUrls, or stateProvider must be specified.");
}
+
return new CloudHttp2SolrClient(this);
}
}
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
index e6602f47545..3ab0d5e844a 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
@@ -178,6 +178,11 @@ public abstract class CloudSolrClient extends SolrClient {
public Builder(List<String> zkHosts, Optional<String> zkChroot) {
super(zkHosts, zkChroot);
}
+
+ /** for an expert use-case */
+ public Builder(ClusterStateProvider stateProvider) {
+ super(stateProvider);
+ }
}
static class StateCache extends ConcurrentHashMap<String,
ExpiringCachedDocCollection> {
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 19c32d9c138..01f57970e99 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
@@ -17,6 +17,7 @@
package org.apache.solr.client.solrj.impl;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -30,7 +31,6 @@ import java.util.Optional;
import org.apache.solr.cloud.SolrCloudTestCase;
import org.junit.BeforeClass;
import org.junit.Test;
-import org.mockito.Mockito;
public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase {
private static final String ANY_CHROOT = "/ANY_CHROOT";
@@ -128,23 +128,23 @@ public class CloudHttp2SolrClientBuilderTest extends
SolrCloudTestCase {
() ->
new CloudHttp2SolrClient.Builder(
Collections.singletonList(ANY_ZK_HOST),
Optional.of(ANY_CHROOT))
- .withHttpClient(Mockito.mock(Http2SolrClient.class))
-
.withInternalClientBuilder(Mockito.mock(Http2SolrClient.Builder.class))
+ .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(Mockito.mock(Http2SolrClient.Builder.class))
- .withHttpClient(Mockito.mock(Http2SolrClient.class))
+ .withInternalClientBuilder(mock(Http2SolrClient.Builder.class))
+ .withHttpClient(mock(Http2SolrClient.class))
.build());
}
@Test
public void testProvideInternalBuilder() throws IOException {
- Http2SolrClient http2Client = Mockito.mock(Http2SolrClient.class);
- Http2SolrClient.Builder http2ClientBuilder =
Mockito.mock(Http2SolrClient.Builder.class);
+ Http2SolrClient http2Client = mock(Http2SolrClient.class);
+ Http2SolrClient.Builder http2ClientBuilder =
mock(Http2SolrClient.Builder.class);
when(http2ClientBuilder.build()).thenReturn(http2Client);
CloudHttp2SolrClient.Builder clientBuilder =
new CloudHttp2SolrClient.Builder(
@@ -162,7 +162,7 @@ public class CloudHttp2SolrClientBuilderTest extends
SolrCloudTestCase {
@Test
public void testProvideExternalClient() throws IOException {
- Http2SolrClient http2Client = Mockito.mock(Http2SolrClient.class);
+ Http2SolrClient http2Client = mock(Http2SolrClient.class);
CloudHttp2SolrClient.Builder clientBuilder =
new CloudHttp2SolrClient.Builder(
Collections.singletonList(ANY_ZK_HOST),
Optional.of(ANY_CHROOT))
@@ -228,4 +228,12 @@ public class CloudHttp2SolrClientBuilderTest extends
SolrCloudTestCase {
((Http2ClusterStateProvider)
client.getClusterStateProvider()).getHttpClient());
}
}
+
+ public void testCustomStateProvider() throws IOException {
+ ZkClientClusterStateProvider stateProvider =
mock(ZkClientClusterStateProvider.class);
+ try (CloudSolrClient cloudSolrClient = new
CloudSolrClient.Builder(stateProvider).build()) {
+ assertEquals(stateProvider, cloudSolrClient.getClusterStateProvider());
+ }
+ verify(stateProvider, times(1)).close();
+ }
}
