This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit f336f6f3f311200e801ae3d6f71033d0073027e0 Author: Serhiy Bzhezytskyy <[email protected]> AuthorDate: Tue Aug 25 16:19:42 2026 +0300 SOLR-18380: remove CloudSolrClient.connect(), ClusterStateProvider.connect() (#4762) And avoid using the deprecated ZK-specific CSC Builder constructor. (cherry picked from commit 0c77e420e8ff627f7093ad64fa30312949eecc45) --- ...-cloudsolrclient-connect-and-legacy-builder.yml | 8 ++ .../java/org/apache/solr/cli/RunExampleTool.java | 6 +- .../apache/solr/cloud/CreateRoutedAliasTest.java | 3 - .../test/org/apache/solr/cloud/OverseerTest.java | 1 - .../org/apache/solr/cloud/TestStressLiveNodes.java | 3 +- .../cloud/api/collections/TestCollectionAPI.java | 3 - .../api/collections/TestReplicaProperties.java | 1 - .../processor/RoutedAliasUpdateProcessorTest.java | 1 - .../TestSimplePropagatorDistributedTracing.java | 9 +- .../manager/consumer/KafkaCrossDcConsumer.java | 3 +- .../messageprocessor/SolrMessageProcessor.java | 3 +- .../messageprocessor/SolrMessageProcessorTest.java | 7 +- .../examples/UsingSolrJRefGuideExamplesTest.java | 14 +-- .../solr/client/solrj/io/SolrClientCache.java | 3 +- .../apache/solr/client/solrj/io/sql/JdbcTest.java | 1 - .../solrj/impl/ZkClientClusterStateProvider.java | 7 -- .../client/solrj/impl/NodeValueFetcherTest.java | 6 +- .../solrj/impl/BaseHttpClusterStateProvider.java | 5 - .../solr/client/solrj/impl/CloudSolrClient.java | 108 ++++++--------------- .../client/solrj/impl/ClusterStateProvider.java | 3 - .../solrj/impl/DelegatingClusterStateProvider.java | 7 -- .../impl/CloudHttp2SolrClientBuilderTest.java | 54 +++-------- .../CloudHttp2SolrClientMultiConstructorTest.java | 13 ++- .../solrj/impl/CloudHttp2SolrClientRetryTest.java | 6 +- .../solrj/impl/CloudHttp2SolrClientTest.java | 19 ++-- .../solrj/impl/CloudSolrClientRoutingTest.java | 4 +- .../impl/SendUpdatesToLeadersOverrideTest.java | 13 +-- .../solrj/impl/TestCloudSolrClientConnections.java | 15 --- .../src/java/org/apache/solr/SolrTestCaseJ4.java | 5 +- .../solr/cloud/AbstractFullDistribZkTestBase.java | 7 +- .../apache/solr/cloud/MiniSolrCloudCluster.java | 10 +- .../org/apache/solr/cloud/SolrCloudTestCase.java | 3 - 32 files changed, 112 insertions(+), 239 deletions(-) diff --git a/changelog/unreleased/SOLR-18380-remove-cloudsolrclient-connect-and-legacy-builder.yml b/changelog/unreleased/SOLR-18380-remove-cloudsolrclient-connect-and-legacy-builder.yml new file mode 100644 index 00000000000..662c75159a7 --- /dev/null +++ b/changelog/unreleased/SOLR-18380-remove-cloudsolrclient-connect-and-legacy-builder.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Remove the deprecated CloudSolrClient.connect() and connect(long, TimeUnit) methods, ClusterStateProvider.connect(), and the CloudSolrClient.Builder(List, Optional) constructor. Call ClusterStateProvider.getLiveNodes() to force a connection, and build with the connection-string constructor, which also honours a chroot inside the string. +type: removed +authors: + - name: Serhiy Bzhezytskyy +links: + - name: SOLR-18380 + url: https://issues.apache.org/jira/browse/SOLR-18380 diff --git a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java index 8db32fc0b66..dcb33138367 100644 --- a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java +++ b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java @@ -29,10 +29,8 @@ import java.nio.file.StandardCopyOption; import java.time.Duration; import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Optional; import java.util.Scanner; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -673,8 +671,8 @@ public class RunExampleTool extends ToolBase { /** wait until the number of live nodes == numNodes. */ protected void waitToSeeLiveNodes(String zkHost, int numNodes) { - try (CloudSolrClient cloudClient = - new CloudSolrClient.Builder(List.of(zkHost), Optional.empty()).build()) { + // honours a chroot inside zkHost, e.g. zk1:2181/solr + try (CloudSolrClient cloudClient = new CloudSolrClient.Builder(zkHost).build()) { Set<String> liveNodes = cloudClient.getClusterState().getLiveNodes(); int numLiveNodes = (liveNodes != null) ? liveNodes.size() : 0; long timeoutNanos = System.nanoTime() + TimeUnit.NANOSECONDS.convert(10, TimeUnit.SECONDS); diff --git a/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java b/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java index f81ed5de933..9184308b108 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java @@ -545,9 +545,6 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase { } private void assertCollectionExists(String name) { - solrClient.getClusterStateProvider().connect(); // TODO get rid of this - // https://issues.apache.org/jira/browse/SOLR-9784?focusedCommentId=16332729 - assertNotNull(name + " not found", solrClient.getClusterStateProvider().getState(name)); // note: could also do: // List collections = CollectionAdminRequest.listCollections(solrClient); 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 4c0077c2e96..15ae9733f7a 100644 --- a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java @@ -1808,7 +1808,6 @@ public class OverseerTest extends SolrTestCaseJ4 { solrClients.add(cloudSolrClient); solrClients.add(httpSolrClient); SolrClientCloudManager sccm = new SolrClientCloudManager(cloudSolrClient, null); - sccm.getClusterStateProvider().connect(); return sccm; } diff --git a/solr/core/src/test/org/apache/solr/cloud/TestStressLiveNodes.java b/solr/core/src/test/org/apache/solr/cloud/TestStressLiveNodes.java index 956c59b3085..b4a0058a47f 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestStressLiveNodes.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestStressLiveNodes.java @@ -66,7 +66,8 @@ public class TestStressLiveNodes extends SolrCloudTestCase { configureCluster(1).configure(); CLOUD_CLIENT = cluster.getSolrClient(); - CLOUD_CLIENT.connect(); // force connection even though we aren't sending any requests + // force the connection now, even though we aren't sending any requests + CLOUD_CLIENT.getClusterStateProvider().getLiveNodes(); ZK_SERVER_ADDR = cluster.getZkServer().getZkAddress(); } diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java index d0de765bd69..b8c1a0e9a3e 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java @@ -687,7 +687,6 @@ public class TestCollectionAPI extends ReplicaPropertiesBase { private void clusterStatusRolesTest() throws Exception { try (CloudSolrClient client = createCloudClient(null)) { - client.connect(); Replica replica = ZkStateReader.from(client).getLeaderRetry(DEFAULT_COLLECTION, SHARD1); ModifiableSolrParams params = new ModifiableSolrParams(); @@ -737,7 +736,6 @@ public class TestCollectionAPI extends ReplicaPropertiesBase { private void replicaPropTest() throws Exception { try (CloudSolrClient client = createCloudClient(null)) { - client.connect(); Map<String, Slice> slices = client.getClusterState().getCollection(COLLECTION_NAME).getSlicesMap(); List<String> sliceList = new ArrayList<>(slices.keySet()); @@ -1204,7 +1202,6 @@ public class TestCollectionAPI extends ReplicaPropertiesBase { private void testShardCreationNameValidation() throws Exception { try (CloudSolrClient client = createCloudClient(null)) { - client.connect(); // Create a collection w/ implicit router ModifiableSolrParams params = new ModifiableSolrParams(); params.set("action", CollectionParams.CollectionAction.CREATE.toString()); diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestReplicaProperties.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestReplicaProperties.java index b2149c9825e..9564c228c24 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestReplicaProperties.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestReplicaProperties.java @@ -85,7 +85,6 @@ public class TestReplicaProperties extends ReplicaPropertiesBase { private void clusterAssignPropertyTest() throws Exception { try (CloudSolrClient client = createCloudClient(null)) { - client.connect(); SolrException se = assertThrows( SolrException.class, diff --git a/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java index 96d5cfe4c5a..888cba4c4ff 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java @@ -221,7 +221,6 @@ public abstract class RoutedAliasUpdateProcessorTest extends SolrCloudTestCase { void assertRouting(int numShards, List<UpdateCommand> updateCommands) throws IOException { CloudSolrClient cloudSolrClient = cluster.getSolrClient(); ClusterStateProvider clusterStateProvider = cloudSolrClient.getClusterStateProvider(); - clusterStateProvider.connect(); Set<String> leaders = getLeaderCoreNames(clusterStateProvider.getClusterState()); assertEquals( "should have " + 3 * numShards + " leaders, " + numShards + " per collection", diff --git a/solr/core/src/test/org/apache/solr/util/tracing/TestSimplePropagatorDistributedTracing.java b/solr/core/src/test/org/apache/solr/util/tracing/TestSimplePropagatorDistributedTracing.java index 31567090e0c..b7a33257292 100644 --- a/solr/core/src/test/org/apache/solr/util/tracing/TestSimplePropagatorDistributedTracing.java +++ b/solr/core/src/test/org/apache/solr/util/tracing/TestSimplePropagatorDistributedTracing.java @@ -21,8 +21,6 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.Tracer; import java.io.IOException; -import java.util.List; -import java.util.Optional; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.CloudSolrClient; @@ -170,11 +168,8 @@ public class TestSimplePropagatorDistributedTracing extends SolrCloudTestCase { } private CloudSolrClient newCloudSolrClient() { - var builder = - new CloudSolrClient.Builder( - List.of(cluster.getZkServer().getZkAddress()), Optional.empty()); - var client = builder.build(); - client.connect(); + var client = cluster.newSolrClientBuilder().build(); + client.getClusterStateProvider().getLiveNodes(); // force the connection now return client; } diff --git a/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java b/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java index 31c2615e77f..055c16fd9f0 100644 --- a/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java +++ b/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java @@ -21,7 +21,6 @@ import java.time.Duration; import java.util.Arrays; import java.util.Collection; import java.util.List; -import java.util.Optional; import java.util.Properties; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; @@ -130,7 +129,7 @@ public class KafkaCrossDcConsumer extends Consumer.CrossDcConsumer { protected CloudSolrClient createSolrClient() { log.debug("Creating new SolrClient..."); - return new CloudSolrClient.Builder(List.of(zkConnectString), Optional.empty()).build(); + return new CloudSolrClient.Builder(zkConnectString).build(); } @Override diff --git a/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessor.java b/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessor.java index de3484ca45a..98f0bc5e8d4 100644 --- a/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessor.java +++ b/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessor.java @@ -377,7 +377,8 @@ public class SolrMessageProcessor extends MessageProcessor boolean connected = false; while (!connected) { try { - clientSupplier.get().connect(); // volatile null-check if already connected + // forces a ZK round-trip, throwing if the cluster is unreachable + clientSupplier.get().getClusterStateProvider().getLiveNodes(); connected = true; } catch (Exception e) { log.error("Unable to connect to solr server. Not consuming.", e); diff --git a/solr/cross-dc-manager/src/test/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessorTest.java b/solr/cross-dc-manager/src/test/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessorTest.java index e7e26d2e9bf..39f3d43acd8 100644 --- a/solr/cross-dc-manager/src/test/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessorTest.java +++ b/solr/cross-dc-manager/src/test/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessorTest.java @@ -29,6 +29,7 @@ import java.io.IOException; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.ClusterStateProvider; import org.apache.solr.client.solrj.response.SolrResponseBase; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; @@ -45,6 +46,7 @@ import org.junit.Test; public class SolrMessageProcessorTest { private SolrMessageProcessor solrMessageProcessor; private CloudSolrClient client; + private ClusterStateProvider clusterStateProvider; private ResubmitBackoffPolicy resubmitBackoffPolicy; @BeforeClass @@ -55,6 +57,9 @@ public class SolrMessageProcessorTest { @Before public void setUp() { client = mock(CloudSolrClient.class); + // handleItem() probes the cluster through the state provider, so the mock must supply one + clusterStateProvider = mock(ClusterStateProvider.class); + when(client.getClusterStateProvider()).thenReturn(clusterStateProvider); resubmitBackoffPolicy = mock(ResubmitBackoffPolicy.class); solrMessageProcessor = new SolrMessageProcessor(mock(OtelMetrics.class), () -> client, resubmitBackoffPolicy); @@ -134,7 +139,7 @@ public class SolrMessageProcessorTest { solrMessageProcessor.handleItem(mirroredSolrRequest); assertEquals(IQueueHandler.ResultStatus.HANDLED, result.status()); - verify(client, times(1)).connect(); + verify(clusterStateProvider, times(1)).getLiveNodes(); verify(solrRequest, times(1)).process(client); } } diff --git a/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java b/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java index f212c1179a4..9aebf8efb4a 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java +++ b/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Queue; import java.util.UUID; import java.util.concurrent.TimeUnit; @@ -260,21 +259,14 @@ public class UsingSolrJRefGuideExamplesTest extends SolrCloudTestCase { private SolrClient getZookeeperNoRootCloudSolrClient() { // tag::solrj-cloudsolrclient-zookeepernoroot[] - final List<String> zkServers = new ArrayList<>(); - zkServers.add("zookeeper1:2181"); - zkServers.add("zookeeper2:2181"); - zkServers.add("zookeeper3:2181"); - return new CloudSolrClient.Builder(zkServers, Optional.empty()).build(); + return new CloudSolrClient.Builder("zookeeper1:2181,zookeeper2:2181,zookeeper3:2181").build(); // end::solrj-cloudsolrclient-zookeepernoroot[] } private SolrClient getZookeeperRootCloudSolrClient() { // tag::solrj-cloudsolrclient-zookeeperroot[] - final List<String> zkServers = new ArrayList<>(); - zkServers.add("zookeeper1:2181"); - zkServers.add("zookeeper2:2181"); - zkServers.add("zookeeper3:2181"); - return new CloudSolrClient.Builder(zkServers, Optional.of("/solr")).build(); + return new CloudSolrClient.Builder("zookeeper1:2181,zookeeper2:2181,zookeeper3:2181/solr") + .build(); // end::solrj-cloudsolrclient-zookeeperroot[] } diff --git a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java index 3503bdb9079..783b4af6edc 100644 --- a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java +++ b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java @@ -116,7 +116,8 @@ public class SolrClientCache implements Closeable { builder = builder.withHttpClientBuilder(newHttpSolrClientBuilder(null, httpSolrClient)); var client = builder.build(); try { - client.connect(); + // force a connection now, so a bad cluster fails here rather than on first use + client.getClusterStateProvider().getLiveNodes(); } catch (Exception e) { IOUtils.closeQuietly(client); throw e; diff --git a/solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java b/solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java index 7f719871a2c..6506e076ba1 100644 --- a/solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java +++ b/solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java @@ -646,7 +646,6 @@ public class JdbcTest extends SolrCloudTestCase { } CloudSolrClient solrClient = cluster.getSolrClient(); - solrClient.connect(); ZkStateReader zkStateReader = ZkStateReader.from(solrClient); var tables = new TreeSet<String>(zkStateReader.getClusterState().getCollectionNames()); diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java index 0de2184ad5f..4f6f21dff63 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java @@ -147,13 +147,6 @@ public class ZkClientClusterStateProvider : (String) state.get().getProperties().get("policy"); } - @Override - public void connect() { - // Esentially a No-Op, but force a check that we're not closed and the ZkStateReader is - // available... - final ZkStateReader ignored = getZkStateReader(); - } - public ZkStateReader getZkStateReader() { if (isClosed) { // quick check... throw new AlreadyClosedException(); diff --git a/solr/solrj-zookeeper/src/test/org/apache/solr/client/solrj/impl/NodeValueFetcherTest.java b/solr/solrj-zookeeper/src/test/org/apache/solr/client/solrj/impl/NodeValueFetcherTest.java index 2d07fd82964..85ed937bb17 100644 --- a/solr/solrj-zookeeper/src/test/org/apache/solr/client/solrj/impl/NodeValueFetcherTest.java +++ b/solr/solrj-zookeeper/src/test/org/apache/solr/client/solrj/impl/NodeValueFetcherTest.java @@ -16,8 +16,6 @@ */ package org.apache.solr.client.solrj.impl; -import java.util.List; -import java.util.Optional; import java.util.Set; import org.apache.solr.client.solrj.impl.SolrClientNodeStateProvider.RemoteCallCtx; import org.apache.solr.client.solrj.request.CollectionAdminRequest; @@ -65,9 +63,7 @@ public class NodeValueFetcherTest extends SolrCloudTestCase { @Test public void testGetTags() throws Exception { - try (var cloudSolrClient = - new CloudSolrClient.Builder(List.of(cluster.getZkServer().getZkAddress()), Optional.empty()) - .build()) { + try (var cloudSolrClient = cluster.newSolrClientBuilder().build()) { int totalCores = 0; // Sum all the cores of the collection by fetching tags of all nodes. diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java index e2faf6b58ba..51c9f8ee4ab 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java @@ -89,11 +89,6 @@ public abstract class BaseHttpClusterStateProvider implements ClusterStateProvid } } - @Override - public void connect() { - getLiveNodes(); - } - /** Create a SolrClient implementation that uses the specified Solr node URL */ protected abstract SolrClient getSolrClient(String baseUrl); 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 53d759dd8bb..18b6fe01f98 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 @@ -43,7 +43,6 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -236,51 +235,6 @@ public abstract class CloudSolrClient extends SolrClient { return parallelUpdates; } - /** - * Connect to the zookeeper ensemble. This is an optional method that may be used to force a - * connection before any other requests are sent. - * - * @deprecated Call {@link ClusterStateProvider#getLiveNodes()} instead. - */ - @Deprecated - public void connect() { - getClusterStateProvider().connect(); - } - - /** - * Connect to a cluster. If the cluster is not ready, retry connection up to a given timeout. - * - * @param duration the timeout - * @param timeUnit the units of the timeout - * @throws TimeoutException if the cluster is not ready after the timeout - * @throws InterruptedException if the wait is interrupted - */ - @Deprecated - public void connect(long duration, TimeUnit timeUnit) - throws TimeoutException, InterruptedException { - if (log.isInfoEnabled()) { - log.info( - "Waiting for {} {} for cluster at {} to be ready", - duration, - timeUnit, - getClusterStateProvider()); - } - long timeout = System.nanoTime() + timeUnit.toNanos(duration); - while (System.nanoTime() < timeout) { - try { - connect(); - if (log.isInfoEnabled()) { - log.info("Cluster at {} ready", getClusterStateProvider()); - } - return; - } catch (RuntimeException e) { - // not ready yet, then... - } - TimeUnit.MILLISECONDS.sleep(250); - } - throw new TimeoutException("Timed out waiting for cluster"); - } - @SuppressWarnings({"unchecked"}) private NamedList<Object> directUpdate(UpdateRequest request, String collection) throws SolrServerException { @@ -1367,37 +1321,6 @@ public abstract class CloudSolrClient extends SolrClient { this.solrUrls = solrUrls; } - /** - * Provide a series of ZK hosts which will be used when configuring {@link CloudSolrClient} - * instances. - * - * <p>Usage example when Solr stores data at the ZooKeeper root ('/'): - * - * <pre> - * final List<String> zkServers = new ArrayList<String>(); - * zkServers.add("zookeeper1:2181"); zkServers.add("zookeeper2:2181"); zkServers.add("zookeeper3:2181"); - * final SolrClient client = new CloudSolrClient.Builder(zkServers, Optional.empty()).build(); - * </pre> - * - * Usage example when Solr data is stored in a ZooKeeper chroot: - * - * <pre> - * final List<String> zkServers = new ArrayList<String>(); - * zkServers.add("zookeeper1:2181"); zkServers.add("zookeeper2:2181"); zkServers.add("zookeeper3:2181"); - * final SolrClient client = new CloudSolrClient.Builder(zkServers, Optional.of("/solr")).build(); - * </pre> - * - * @param zkHosts a List of at least one ZooKeeper host and port (e.g. "zookeeper1:2181") - * @param zkChroot the path to the root ZooKeeper node containing Solr data. Provide {@code - * java.util.Optional.empty()} if no ZK chroot is used. - * @deprecated Use a connectionString constructor and/or prefer HTTP URLs instead. - */ - @Deprecated(since = "10.1") // sort of 10.0 but accidentally removed - public Builder(List<String> zkHosts, Optional<String> zkChroot) { - this.zkHosts = zkHosts; - if (zkChroot.isPresent()) this.zkChroot = zkChroot.get(); - } - /** for an expert use-case */ public Builder(ClusterStateProvider stateProvider) { this.stateProvider = stateProvider; @@ -1436,6 +1359,37 @@ public abstract class CloudSolrClient extends SolrClient { } } + /** + * Provide a series of ZK hosts which will be used when configuring {@link CloudSolrClient} + * instances. + * + * <p>Usage example when Solr stores data at the ZooKeeper root ('/'): + * + * <pre> + * final List<String> zkServers = new ArrayList<String>(); + * zkServers.add("zookeeper1:2181"); zkServers.add("zookeeper2:2181"); zkServers.add("zookeeper3:2181"); + * final SolrClient client = new CloudSolrClient.Builder(zkServers, Optional.empty()).build(); + * </pre> + * + * Usage example when Solr data is stored in a ZooKeeper chroot: + * + * <pre> + * final List<String> zkServers = new ArrayList<String>(); + * zkServers.add("zookeeper1:2181"); zkServers.add("zookeeper2:2181"); zkServers.add("zookeeper3:2181"); + * final SolrClient client = new CloudSolrClient.Builder(zkServers, Optional.of("/solr")).build(); + * </pre> + * + * @param zkHosts a List of at least one ZooKeeper host and port (e.g. "zookeeper1:2181") + * @param zkChroot the path to the root ZooKeeper node containing Solr data. Provide {@code + * java.util.Optional.empty()} if no ZK chroot is used. + * @deprecated Use a connectionString constructor and/or prefer HTTP URLs instead. + */ + @Deprecated(since = "10.1") // sort of 10.0 but accidentally removed + public Builder(List<String> zkHosts, Optional<String> zkChroot) { + this.zkHosts = zkHosts; + if (zkChroot.isPresent()) this.zkChroot = zkChroot.get(); + } + /** Whether to use the default ZK ACLs when building a ZK Client. */ public Builder canUseZkACLs(boolean canUseZkACLs) { this.canUseZkACLs = canUseZkACLs; diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java index 439363f9149..bf830b1eb2e 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java @@ -123,9 +123,6 @@ public interface ClusterStateProvider extends SolrCloseable { /** Get the collection-specific policy */ String getPolicyNameByCollection(String coll); - @Deprecated // just call getLiveNodes() - void connect(); - String getQuorumHosts(); /** Get url scheme like http or https but never null. */ diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegatingClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegatingClusterStateProvider.java index 6ffd111e761..d3a32f5fc8c 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegatingClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegatingClusterStateProvider.java @@ -118,13 +118,6 @@ public class DelegatingClusterStateProvider implements ClusterStateProvider { return cs == null ? null : cs.getCollectionOrNull(name); } - @Override - public void connect() { - if (delegate != null) { - delegate.connect(); - } - } - @Override public void close() throws IOException { if (delegate != null) { 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 62be902fa67..b7e2ea826e8 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 @@ -24,9 +24,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.IOException; -import java.util.ArrayList; import java.util.List; -import java.util.Optional; import java.util.concurrent.TimeUnit; import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; import org.apache.solr.cloud.SolrCloudTestCase; @@ -55,7 +53,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { @Test public void testSingleZkHostSpecified() throws IOException { try (CloudSolrClient createdClient = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -66,35 +64,17 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { } @Test - public void testSeveralZkHostsSpecifiedSingly() throws IOException { - final List<String> zkHostList = new ArrayList<>(); - zkHostList.add(ANY_ZK_HOST); - zkHostList.add(ANY_OTHER_ZK_HOST); + public void testSeveralZkHostsWithChroot() throws IOException { + final List<String> zkHostList = List.of(ANY_ZK_HOST, ANY_OTHER_ZK_HOST); try (CloudSolrClient createdClient = - new CloudSolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { - try (ZkClientClusterStateProvider zkClientClusterStateProvider = - ZkClientClusterStateProvider.from(createdClient)) { - final String clientZkHost = zkClientClusterStateProvider.getZkHost(); - - assertTrue(clientZkHost.contains(ANY_ZK_HOST)); - assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST)); - } - } - } - - @Test - public void testSeveralZkHostsSpecifiedTogether() throws IOException { - final ArrayList<String> zkHosts = new ArrayList<>(); - zkHosts.add(ANY_ZK_HOST); - zkHosts.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = - new CloudSolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { + new CloudSolrClient.Builder(String.join(",", zkHostList) + ANY_CHROOT).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); assertTrue(clientZkHost.contains(ANY_ZK_HOST)); assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST)); + assertTrue(clientZkHost.endsWith(ANY_CHROOT)); } } } @@ -102,7 +82,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { @Test public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throws IOException { try (CloudSolrClient createdClient = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT).build()) { assertTrue(createdClient.isUpdatesToLeaders()); } } @@ -110,7 +90,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { @Test public void testIsDirectUpdatesToLeadersOnlyDefault() throws IOException { try (CloudSolrClient createdClient = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT).build()) { assertFalse(createdClient.isDirectUpdatesToLeadersOnly()); } } @@ -120,14 +100,14 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { expectThrows( IllegalStateException.class, () -> - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT) .withHttpClient(mock(HttpJettySolrClient.class)) .withHttpClientBuilder(mock(HttpJettySolrClient.Builder.class)) .build()); expectThrows( IllegalStateException.class, () -> - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT) .withHttpClientBuilder(mock(HttpJettySolrClient.Builder.class)) .withHttpClient(mock(HttpJettySolrClient.class)) .build()); @@ -139,7 +119,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { HttpJettySolrClient.Builder http2ClientBuilder = mock(HttpJettySolrClient.Builder.class); when(http2ClientBuilder.build()).thenReturn(http2Client); CloudSolrClient.Builder clientBuilder = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT) .withHttpClientBuilder(http2ClientBuilder); verify(http2ClientBuilder, never()).build(); try (CloudHttp2SolrClient client = clientBuilder.build()) { @@ -157,7 +137,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { HttpJdkSolrClient.Builder http2ClientBuilder = mock(HttpJdkSolrClient.Builder.class); when(http2ClientBuilder.build()).thenReturn(http2Client); CloudSolrClient.Builder clientBuilder = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT) .withHttpClientBuilder(http2ClientBuilder); verify(http2ClientBuilder, never()).build(); try (CloudHttp2SolrClient client = clientBuilder.build()) { @@ -173,8 +153,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { public void testProvideExternalJettyClient() throws IOException { HttpJettySolrClient http2Client = mock(HttpJettySolrClient.class); CloudSolrClient.Builder clientBuilder = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) - .withHttpClient(http2Client); + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT).withHttpClient(http2Client); try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); } @@ -186,8 +165,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { public void testProvideExternalJdkClient() throws IOException { HttpJdkSolrClient http2Client = mock(HttpJdkSolrClient.class); CloudSolrClient.Builder clientBuilder = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) - .withHttpClient(http2Client); + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT).withHttpClient(http2Client); try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); } @@ -198,7 +176,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { @Test public void testDefaultClientUsesJetty() throws IOException { try (CloudHttp2SolrClient createdClient = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT).build()) { assertTrue(createdClient.getHttpClient() instanceof HttpJettySolrClient); assertTrue(createdClient.getLbClient().getClient(null) instanceof HttpJettySolrClient); } @@ -207,7 +185,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { @Test public void testDefaultCollectionPassedFromBuilderToClient() throws IOException { try (CloudHttp2SolrClient createdClient = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudSolrClient.Builder(ANY_ZK_HOST + ANY_CHROOT) .withDefaultCollection("aCollection") .build()) { assertEquals("aCollection", createdClient.getDefaultCollection()); @@ -270,7 +248,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { @SuppressWarnings({"try"}) public void test0Timeouts() throws IOException { try (CloudSolrClient createdClient = - new CloudSolrClient.Builder(List.of(ANY_ZK_HOST), Optional.empty()) + new CloudSolrClient.Builder(ANY_ZK_HOST) .withZkConnectTimeout(0, TimeUnit.MILLISECONDS) .withZkClientTimeout(0, TimeUnit.MILLISECONDS) .build()) { diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientMultiConstructorTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientMultiConstructorTest.java index 5442d705860..83d9e55701a 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientMultiConstructorTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientMultiConstructorTest.java @@ -21,7 +21,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashSet; import java.util.List; -import java.util.Optional; import org.apache.lucene.tests.util.TestUtil; import org.apache.solr.SolrTestCase; import org.junit.Test; @@ -68,9 +67,11 @@ public class CloudHttp2SolrClientMultiConstructorTest extends SolrTestCase { clientChroot = "/mychroot"; } - try (CloudSolrClient client = - new CloudSolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) - .build()) { + // the parser takes everything from the first '/' as the chroot + final String connectionString = + String.join(",", hosts) + (clientChroot == null ? "" : clientChroot); + + try (CloudSolrClient client = new CloudSolrClient.Builder(connectionString).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(client)) { assertEquals(sb.toString(), zkClientClusterStateProvider.getZkHost()); @@ -82,6 +83,8 @@ public class CloudHttp2SolrClientMultiConstructorTest extends SolrTestCase { public void testBadChroot() { final List<String> zkHosts = new ArrayList<>(); zkHosts.add("host1:2181"); - new CloudSolrClient.Builder(zkHosts, Optional.of("foo")).build(); + // "foo" has no leading slash, so it can only reach validation via the connection record + new CloudSolrClient.Builder(new CloudSolrClient.CloudSolrClientConnection(true, zkHosts, "foo")) + .build(); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index c5f52ccaa61..c240d6e8e6f 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -19,8 +19,6 @@ package org.apache.solr.client.solrj.impl; import static org.apache.solr.util.SolrJMetricTestUtils.getPrometheusMetricValue; -import java.util.List; -import java.util.Optional; import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.cloud.SolrCloudTestCase; @@ -59,9 +57,7 @@ public class CloudHttp2SolrClientRetryTest extends SolrCloudTestCase { .useHttp1_1(true) .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); - var cloudSolrclientBuilder = - new CloudSolrClient.Builder( - List.of(cluster.getZkServer().getZkAddress()), Optional.empty()); + var cloudSolrclientBuilder = cluster.newSolrClientBuilder(); cloudSolrclientBuilder.withHttpClientBuilder( random().nextBoolean() ? jettyClientBuilder : jdkClientBuilder); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java index 88ae42fc32c..bb8ca9898e4 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java @@ -102,7 +102,6 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { private static CloudHttp2SolrClient httpJettyBasedCloudSolrClient = null; private static CloudHttp2SolrClient httpJdkBasedCloudSolrClient = null; - private static CloudHttp2SolrClient zkBasedCloudSolrClient = null; private static CloudHttp2SolrClient connectionStringZkBasedCloudSolrClient = null; private static CloudHttp2SolrClient connectionStringHttpBasedCloudSolrClient = null; @@ -154,13 +153,6 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { .getHttpClient() instanceof HttpJdkSolrClient); - zkBasedCloudSolrClient = - new CloudSolrClient.Builder(List.of(cluster.getZkServer().getZkAddress()), Optional.empty()) - .build(); - assertTrue(zkBasedCloudSolrClient.getHttpClient() instanceof HttpJettySolrClient); - assertTrue( - zkBasedCloudSolrClient.getClusterStateProvider() instanceof ZkClientClusterStateProvider); - String zkConnString = cluster.getZkServer().getZkAddress(); connectionStringZkBasedCloudSolrClient = new CloudSolrClient.Builder(zkConnString).build(); assertTrue( @@ -182,14 +174,12 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { public static void tearDownAfterClass() throws Exception { IOUtils.closeQuietly(httpJettyBasedCloudSolrClient); IOUtils.closeQuietly(httpJdkBasedCloudSolrClient); - IOUtils.closeQuietly(zkBasedCloudSolrClient); IOUtils.closeQuietly(connectionStringZkBasedCloudSolrClient); IOUtils.closeQuietly(connectionStringHttpBasedCloudSolrClient); shutdownCluster(); httpJettyBasedCloudSolrClient = null; httpJdkBasedCloudSolrClient = null; - zkBasedCloudSolrClient = null; connectionStringZkBasedCloudSolrClient = null; connectionStringHttpBasedCloudSolrClient = null; } @@ -197,7 +187,6 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { /** Randomly return the cluster's ZK based CSC, or HttpClusterProvider based CSC. */ private CloudSolrClient getRandomClient() { CloudSolrClient[] clients = { - zkBasedCloudSolrClient, httpJettyBasedCloudSolrClient, httpJdkBasedCloudSolrClient, connectionStringZkBasedCloudSolrClient, @@ -915,7 +904,9 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(client)) { zkClientClusterStateProvider.setZkConnectTimeout(100); - SolrException e = assertThrows(SolrException.class, client::connect); + SolrException e = + assertThrows( + SolrException.class, () -> client.getClusterStateProvider().getLiveNodes()); assertTrue(e.getCause() instanceof TimeoutException); } } @@ -930,7 +921,9 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(client)) { zkClientClusterStateProvider.setZkClientTimeout(1000 * 60); - SolrException e = assertThrows(SolrException.class, client::connect); + SolrException e = + assertThrows( + SolrException.class, () -> client.getClusterStateProvider().getLiveNodes()); assertTrue(e.getMessage().contains("cluster not found/not ready")); assertTrue( e.getMessage() diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java index 69bc456885f..328ff2437be 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRoutingTest.java @@ -19,7 +19,6 @@ package org.apache.solr.client.solrj.impl; import java.io.IOException; import java.util.ArrayList; import java.util.List; -import java.util.Optional; import org.apache.lucene.tests.util.TestUtil; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; @@ -148,7 +147,8 @@ public final class CloudSolrClientRoutingTest extends SolrCloudTestCase { cluster.waitForActiveCollection(collectionName, 2, 2); try (CloudSolrClient client = - new CloudSolrClient.Builder(List.of(cluster.getZkServer().getZkAddress()), Optional.empty()) + cluster + .newSolrClientBuilder() .withDefaultCollection(collectionName) .sendUpdatesOnlyToShardLeaders() .sendDirectUpdatesToShardLeadersOnly() diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java index ab27c827c62..366a048a8a5 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java @@ -27,7 +27,6 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.apache.solr.client.solrj.SolrClient; @@ -206,18 +205,14 @@ public class SendUpdatesToLeadersOverrideTest extends SolrCloudTestCase { } public void testBuilderImplicitBehavior() throws Exception { - try (CloudSolrClient client = - new CloudSolrClient.Builder(List.of(cluster.getZkServer().getZkAddress()), Optional.empty()) - .build()) { + try (CloudSolrClient client = cluster.newSolrClientBuilder().build()) { assertTrue(client.isUpdatesToLeaders()); } } public void testHttp2ClientThatDefaultsToLeaders() throws Exception { try (CloudSolrClient client = - new CloudSolrClient.Builder(List.of(cluster.getZkServer().getZkAddress()), Optional.empty()) - .sendUpdatesOnlyToShardLeaders() - .build()) { + cluster.newSolrClientBuilder().sendUpdatesOnlyToShardLeaders().build()) { checkUpdatesDefaultToLeaders(client); checkUpdatesWithSendToLeadersFalse(client); } @@ -225,9 +220,7 @@ public class SendUpdatesToLeadersOverrideTest extends SolrCloudTestCase { public void testHttp2ClientThatDoesNotDefaultToLeaders() throws Exception { try (CloudSolrClient client = - new CloudSolrClient.Builder(List.of(cluster.getZkServer().getZkAddress()), Optional.empty()) - .sendUpdatesToAnyReplica() - .build()) { + cluster.newSolrClientBuilder().sendUpdatesToAnyReplica().build()) { checkUpdatesWithShardsPrefPull(client); checkUpdatesWithSendToLeadersFalse(client); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/TestCloudSolrClientConnections.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/TestCloudSolrClientConnections.java index 19c1a75de39..eeeccab9096 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/TestCloudSolrClientConnections.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/TestCloudSolrClientConnections.java @@ -17,7 +17,6 @@ package org.apache.solr.client.solrj.impl; import java.nio.file.Path; -import java.util.concurrent.TimeUnit; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.cloud.MiniSolrCloudCluster; @@ -47,7 +46,6 @@ public class TestCloudSolrClientConnections extends SolrTestCaseJ4 { cluster.startJettySolrRunner(); cluster.waitForAllNodes(30); - client.connect(20, TimeUnit.SECONDS); // should work now! client.request(listReq); @@ -65,7 +63,6 @@ public class TestCloudSolrClientConnections extends SolrTestCaseJ4 { MiniSolrCloudCluster cluster = new MiniSolrCloudCluster(0, createTempDir(), JettyConfig.builder().build()); try { - CloudSolrClient client = cluster.getSolrClient(); SolrException e = expectThrows( SolrException.class, @@ -78,7 +75,6 @@ public class TestCloudSolrClientConnections extends SolrTestCaseJ4 { cluster.startJettySolrRunner(); cluster.waitForAllNodes(30); - client.connect(20, TimeUnit.SECONDS); cluster.getZkClient().upConfig(configPath, "testconfig"); @@ -125,21 +121,10 @@ public class TestCloudSolrClientConnections extends SolrTestCaseJ4 { /** NOTE: will close the provider and assert it starts throwing AlreadyClosedException */ private void checkAndCloseProvider(final ZkClientClusterStateProvider provider) throws Exception { - if (random().nextBoolean()) { - // calling connect should be purely optional and affect nothing - provider.connect(); - } assertNotNull(provider.getClusterState()); provider.close(); - if (random().nextBoolean()) { - expectThrows( - AlreadyClosedException.class, - () -> { - provider.connect(); - }); - } expectThrows( AlreadyClosedException.class, () -> { diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index 3ea838e7754..0a381196312 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java @@ -2479,7 +2479,10 @@ public abstract class SolrTestCaseJ4 extends SolrTestCase { public static class RandomizingCloudSolrClientBuilder extends CloudSolrClient.Builder { public RandomizingCloudSolrClientBuilder(List<String> zkHosts, Optional<String> zkChroot) { - super(zkHosts, zkChroot); + // sets the protected fields directly, matching the sibling constructors below + super(new ArrayList<>()); + this.zkHosts.addAll(zkHosts); + zkChroot.ifPresent(chroot -> this.zkChroot = chroot); randomizeCloudSolrClient(); } diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java index 1d98717c67d..5a3afd70ed0 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java @@ -355,7 +355,6 @@ public abstract class AbstractFullDistribZkTestBase extends BaseDistributedSearc assert (cloudInit == false); cloudInit = true; cloudClient = createCloudClient(DEFAULT_COLLECTION); - cloudClient.connect(); ZkStateReader zkStateReader = ZkStateReader.from(cloudClient); @@ -405,7 +404,7 @@ public abstract class AbstractFullDistribZkTestBase extends BaseDistributedSearc // create the normal cloud client. // this can change if more tests need it. controlClientCloud = createCloudClient("control_collection"); - controlClientCloud.connect(); + controlClientCloud.getClusterStateProvider().getLiveNodes(); // force the connection // NOTE: we are skipping creation of the chaos monkey by returning here cloudClient = controlClientCloud; // temporary - some code needs/uses // cloudClient @@ -2545,7 +2544,7 @@ public abstract class AbstractFullDistribZkTestBase extends BaseDistributedSearc commonCloudSolrClient = createNewCloudSolrClient( zkServer.getZkAddress(), DEFAULT_COLLECTION, random().nextBoolean(), 5000, 120000); - commonCloudSolrClient.connect(); + commonCloudSolrClient.getClusterStateProvider().getLiveNodes(); // force it now if (log.isInfoEnabled()) { log.info( "Created commonCloudSolrClient with updatesToLeaders={} and parallelUpdates={}", @@ -2565,7 +2564,7 @@ public abstract class AbstractFullDistribZkTestBase extends BaseDistributedSearc createNewCloudSolrClient( zkServer.getZkAddress(), collectionName, random().nextBoolean(), 5000, 120000); - solrClient.connect(); + solrClient.getClusterStateProvider().getLiveNodes(); // force the connection now if (log.isInfoEnabled()) { log.info( "Created solrClient for collection {} with updatesToLeaders={} and parallelUpdates={}", diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java index b1cbb83362d..bc56d66cd6e 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java @@ -668,7 +668,7 @@ public class MiniSolrCloudCluster implements SolrBackend { k -> { CloudSolrClient solrClient = newSolrClient(collectionName); - solrClient.connect(); + solrClient.getClusterStateProvider().getLiveNodes(); // force the connection now if (log.isInfoEnabled()) { log.info( "Created solrClient for collection {} with updatesToLeaders={} and parallelUpdates={}", @@ -692,6 +692,14 @@ public class MiniSolrCloudCluster implements SolrBackend { .build(); // we choose 90 because we run in some harsh envs } + /** + * Returns a new {@link org.apache.solr.client.solrj.impl.CloudSolrClient.Builder} pointed at this + * cluster. + */ + public CloudSolrClient.Builder newSolrClientBuilder() { + return new CloudSolrClient.Builder(getZkServer().getZkAddress()); + } + public SolrZkClient getZkClient() { return getZkStateReader().getZkClient(); } diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java index 0e5189d1f8e..dbca1f97ef4 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java @@ -56,7 +56,6 @@ import org.apache.solr.common.cloud.LiveNodesPredicate; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.SolrZkClient; -import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.EnvUtils; import org.apache.solr.common.util.NamedList; import org.apache.solr.embedded.JettySolrRunner; @@ -95,8 +94,6 @@ public class SolrCloudTestCase extends SolrTestCaseJ4 { protected static volatile MiniSolrCloudCluster cluster; protected static SolrZkClient zkClient() { - ZkStateReader reader = cluster.getZkStateReader(); - if (reader == null) cluster.getSolrClient().connect(); return cluster.getZkStateReader().getZkClient(); }
