SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState (#2363)
ZkClientClusterStateProvider needed to double-check a collection truly doesn't exist. HttpClusterStateProvider is already correct.
HttpSolrCall on the server side was hardened similarly.
This could happen for a highly burdened cluster / zookeeper.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 107f7a9..8e7c179 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -135,6 +135,8 @@
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)
diff --git a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
index 4f17f3e..20cec8f 100644
--- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
+++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
@@ -35,14 +35,12 @@
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;
@@ -140,8 +138,20 @@
if (pathSegments.size() > 1 && ("c".equals(prefix) || "collections".equals(prefix))) {
origCorename = pathSegments.get(1);
- DocCollection collection =
- resolveDocCollection(queryParams.get(COLLECTION_PROP, origCorename));
+ 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);
if (collection == null) {
if (!path.endsWith(CommonParams.INTROSPECT)) {
throw new SolrException(
@@ -218,54 +228,6 @@
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,
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index e93c412..2e5856f 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -53,6 +53,7 @@
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;
@@ -281,6 +282,8 @@
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);
@@ -345,6 +348,37 @@
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)
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 f9d202f..bc56881 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
@@ -144,11 +144,22 @@
@Override
public ClusterState.CollectionRef getState(String collection) {
ClusterState clusterState = getZkStateReader().getClusterState();
- if (clusterState != null) {
- return clusterState.getCollectionRef(collection);
- } else {
+ 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;
+ }
+ } else {
+ return collectionRef;
+ }
}
@Override
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 e6b7f20..81bb885 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
@@ -53,7 +53,7 @@
/**
* Obtain the state of the collection (cluster status).
*
- * @return the collection state, or null is collection doesn't exist
+ * @return the collection state, or null only if collection doesn't exist
*/
ClusterState.CollectionRef getState(String collection);
