SOLR-18380: remove CloudSolrClient.connect(), ClusterStateProvider.connect() and the legacy Builder

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

solr/core/src/java/org/apache/solr/cli/RunExampleTool.java

@@ -29,10 +29,8 @@
 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 @@ protected void runCloudExample(CommandLine cli) throws Exception {
 
   /** 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);

solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java

@@ -545,9 +545,6 @@ private void assertFailure(Request req, String expectedErrorSubstring) throws IO
   }
 
   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);

solr/core/src/test/org/apache/solr/cloud/OverseerTest.java

@@ -1808,7 +1808,6 @@ private SolrCloudManager getCloudDataProvider(ZkStateReader zkStateReader) {
     solrClients.add(cloudSolrClient);
     solrClients.add(httpSolrClient);
     SolrClientCloudManager sccm = new SolrClientCloudManager(cloudSolrClient, null);
-    sccm.getClusterStateProvider().connect();
     return sccm;
   }
 

solr/core/src/test/org/apache/solr/cloud/TestStressLiveNodes.java

@@ -66,7 +66,8 @@ public static void createMiniSolrCloudCluster() throws Exception {
     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();
   }

solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java

@@ -687,7 +687,6 @@ private void clusterStatusWithCollectionAndShardJSON() throws IOException, SolrS
 
   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 @@ private void clusterStatusBadCollectionTest() throws Exception {
 
   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 @@ private void testAliasCreationNameValidation() throws Exception {
 
   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());

solr/core/src/test/org/apache/solr/cloud/api/collections/TestReplicaProperties.java

@@ -85,7 +85,6 @@ private void listCollection() throws IOException, SolrServerException {
   private void clusterAssignPropertyTest() throws Exception {
 
     try (CloudSolrClient client = createCloudClient(null)) {
-      client.connect();
       SolrException se =
           assertThrows(
               SolrException.class,

solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java

@@ -221,7 +221,6 @@ Set<String> getLeaderCoreNames(ClusterState clusterState) {
   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",

solr/core/src/test/org/apache/solr/util/tracing/TestSimplePropagatorDistributedTracing.java

@@ -21,8 +21,6 @@
 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 @@ private void assertSameTraceId(LogListener reqLog, String traceId) {
   }
 
   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;
   }
 

solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java

@@ -21,7 +21,6 @@
 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 void close() {
 
     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

solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessor.java

@@ -377,7 +377,8 @@ private void connectToSolrIfNeeded() {
     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);

solr/cross-dc-manager/src/test/org/apache/solr/crossdc/manager/messageprocessor/SolrMessageProcessorTest.java

@@ -29,6 +29,7 @@
 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 @@
 public class SolrMessageProcessorTest {
   private SolrMessageProcessor solrMessageProcessor;
   private CloudSolrClient client;
+  private ClusterStateProvider clusterStateProvider;
   private ResubmitBackoffPolicy resubmitBackoffPolicy;
 
   @BeforeClass
@@ -55,6 +57,9 @@ public static void ensureWorkingMockito() {
   @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 void connectToSolrIfNeededAndProcessRequest() throws SolrServerException,
         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);
   }
 }

solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java

@@ -22,7 +22,6 @@
 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 @@ private SolrClient getBaseURLCloudSolrClient() {
 
   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[]
   }
 

solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java

@@ -116,7 +116,8 @@ protected CloudSolrClient newCloudSolrClient(
     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;

solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java

@@ -646,7 +646,6 @@ private void testJDBCMethods(
       }
 
       CloudSolrClient solrClient = cluster.getSolrClient();
-      solrClient.connect();
       ZkStateReader zkStateReader = ZkStateReader.from(solrClient);
 
       var tables = new TreeSet<String>(zkStateReader.getClusterState().getCollectionNames());

solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java

@@ -147,13 +147,6 @@ public String getPolicyNameByCollection(String coll) {
         : (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();

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 void cleanup() throws Exception {
 
   @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.

solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java

@@ -89,11 +89,6 @@ private static URL stringToUrl(String solrUrl) {
     }
   }
 
-  @Override
-  public void connect() {
-    getLiveNodes();
-  }
-
   /** Create a SolrClient implementation that uses the specified Solr node URL */
   protected abstract SolrClient getSolrClient(String baseUrl);
 

solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java

@@ -33,7 +33,6 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
@@ -43,7 +42,6 @@
 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 +234,6 @@ public boolean isParallelUpdates() {
     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 +1320,6 @@ public Builder(List<String> solrUrls) {
       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&lt;String&gt; zkServers = new ArrayList&lt;String&gt;();
-     *   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&lt;String&gt; zkServers = new ArrayList&lt;String&gt;();
-     *    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;

solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java

@@ -123,9 +123,6 @@ default <T> T getClusterProperty(String key, T defaultValue) {
   /** 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. */