Revert "SOLR-17153: CloudSolrClient should not throw "Collection not …

…found" with an out-dated ClusterState (apache#2363)"

This reverts commit 5c399dd.

solr/CHANGES.txt

@@ -137,8 +137,6 @@ Optimizations
 
 Bug Fixes
 ---------------------
-* SOLR-17153: CloudSolrClient could fail a request immediately following a collection creation.  Required double-checking the collection doesn’t exist. (Aparna Suresh via David Smiley)
-
 * SOLR-17152: Better alignment of Admin UI graph (janhoy)
 
 * SOLR-17148: Fixing Config API overlay property enabling or disabling the cache (Sanjay Dutt, hossman, Eric Pugh)

solr/core/src/java/org/apache/solr/api/V2HttpCall.java

@@ -35,12 +35,14 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Supplier;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import net.jcip.annotations.ThreadSafe;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.util.JsonSchemaValidator;
 import org.apache.solr.common.util.PathTrie;
@@ -138,20 +140,8 @@ public void call(SolrQueryRequest req, SolrQueryResponse rsp) {
       if (pathSegments.size() > 1 && ("c".equals(prefix) || "collections".equals(prefix))) {
         origCorename = pathSegments.get(1);
 
-        String collectionStr = queryParams.get(COLLECTION_PROP, origCorename);
-        collectionsList =
-            resolveCollectionListOrAlias(collectionStr); // &collection= takes precedence
-        if (collectionsList.size() > 1) {
-          throw new SolrException(
-              SolrException.ErrorCode.BAD_REQUEST,
-              "Request must be sent to a single collection "
-                  + "or an alias that points to a single collection,"
-                  + " but '"
-                  + collectionStr
-                  + "' resolves to "
-                  + this.collectionsList);
-        }
-        DocCollection collection = resolveDocCollection(collectionsList);
+        DocCollection collection =
+            resolveDocCollection(queryParams.get(COLLECTION_PROP, origCorename));
         if (collection == null) {
           if (!path.endsWith(CommonParams.INTROSPECT)) {
             throw new SolrException(
@@ -228,6 +218,54 @@ protected void parseRequest() throws Exception {
     if (solrReq == null) solrReq = parser.parse(core, path, req);
   }
 
+  /**
+   * Lookup the collection from the collection string (maybe comma delimited). Also sets {@link
+   * #collectionsList} by side-effect. if {@code secondTry} is false then we'll potentially
+   * recursively try this all one more time while ensuring the alias and collection info is sync'ed
+   * from ZK.
+   */
+  protected DocCollection resolveDocCollection(String collectionStr) {
+    if (!cores.isZooKeeperAware()) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST, "Solr not running in cloud mode ");
+    }
+    ZkStateReader zkStateReader = cores.getZkController().getZkStateReader();
+
+    Supplier<DocCollection> logic =
+        () -> {
+          this.collectionsList = resolveCollectionListOrAlias(collectionStr); // side-effect
+          if (collectionsList.size() > 1) {
+            throw new SolrException(
+                SolrException.ErrorCode.BAD_REQUEST,
+                "Request must be sent to a single collection "
+                    + "or an alias that points to a single collection,"
+                    + " but '"
+                    + collectionStr
+                    + "' resolves to "
+                    + this.collectionsList);
+          }
+          String collectionName = collectionsList.get(0); // first
+          // TODO an option to choose another collection in the list if can't find a local replica
+          // of the first?
+
+          return zkStateReader.getClusterState().getCollectionOrNull(collectionName);
+        };
+
+    DocCollection docCollection = logic.get();
+    if (docCollection != null) {
+      return docCollection;
+    }
+    // ensure our view is up to date before trying again
+    try {
+      zkStateReader.aliasesManager.update();
+      zkStateReader.forceUpdateCollection(collectionsList.get(0));
+    } catch (Exception e) {
+      log.error("Error trying to update state while resolving collection.", e);
+      // don't propagate exception on purpose
+    }
+    return logic.get();
+  }
+
   public static Api getApiInfo(
       PluginBag<SolrRequestHandler> requestHandlers,
       String path,

solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java

@@ -53,7 +53,6 @@
 import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
-import java.util.function.Supplier;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import net.jcip.annotations.ThreadSafe;
@@ -282,8 +281,6 @@ protected void init() throws Exception {
               queryParams.get(COLLECTION_PROP, def)); // &collection= takes precedence
 
       if (core == null) {
-        // force update collection only if local clusterstate is outdated
-        resolveDocCollection(collectionsList);
         // lookup core from collection, or route away if need to
         // route to 1st
         String collectionName = collectionsList.isEmpty() ? null : collectionsList.get(0);
@@ -348,37 +345,6 @@ protected void init() throws Exception {
     action = PASSTHROUGH;
   }
 
-  /**
-   * Lookup the collection from the collection string (maybe comma delimited). Also sets {@link
-   * #collectionsList} by side-effect. if {@code secondTry} is false then we'll potentially
-   * recursively try this all one more time while ensuring the alias and collection info is sync'ed
-   * from ZK.
-   */
-  protected DocCollection resolveDocCollection(List<String> collectionsList) {
-    if (!cores.isZooKeeperAware()) {
-      throw new SolrException(
-          SolrException.ErrorCode.BAD_REQUEST, "Solr not running in cloud mode ");
-    }
-    ZkStateReader zkStateReader = cores.getZkController().getZkStateReader();
-    String collectionName = collectionsList.get(0);
-    Supplier<DocCollection> logic =
-        () -> zkStateReader.getClusterState().getCollectionOrNull(collectionName);
-
-    DocCollection docCollection = logic.get();
-    if (docCollection != null) {
-      return docCollection;
-    }
-    // ensure our view is up to date before trying again
-    try {
-      zkStateReader.aliasesManager.update();
-      zkStateReader.forceUpdateCollection(collectionName);
-    } catch (Exception e) {
-      log.error("Error trying to update state while resolving collection.", e);
-      // don't propagate exception on purpose
-    }
-    return logic.get();
-  }
-
   protected void autoCreateSystemColl(String corename) throws Exception {
     if (core == null
         && SYSTEM_COLL.equals(corename)

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

@@ -144,21 +144,10 @@ public static ClusterState createFromJsonSupportingLegacyConfigName(
   @Override
   public ClusterState.CollectionRef getState(String collection) {
     ClusterState clusterState = getZkStateReader().getClusterState();
-    if (clusterState == null) {
-      return null;
-    }
-
-    ClusterState.CollectionRef collectionRef = clusterState.getCollectionRef(collection);
-    if (collectionRef == null) {
-      // force update collection
-      try {
-        getZkStateReader().forceUpdateCollection(collection);
-        return getZkStateReader().getClusterState().getCollectionRef(collection);
-      } catch (KeeperException | InterruptedException e) {
-        return null;
-      }
+    if (clusterState != null) {
+      return clusterState.getCollectionRef(collection);
     } else {
-      return collectionRef;
+      return null;
     }
   }
 

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

@@ -53,7 +53,7 @@ static ClusterStateProvider newZkClusterStateProvider(
   /**
    * Obtain the state of the collection (cluster status).
    *
-   * @return the collection state, or null only if collection doesn't exist
+   * @return the collection state, or null is collection doesn't exist
    */
   ClusterState.CollectionRef getState(String collection);