This is an automated email from the ASF dual-hosted git repository.
dsmiley 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 8cf552aa364 SOLR-17153: CloudSolrClient should not throw "Collection
not found" with an out-dated ClusterState (#2363)
8cf552aa364 is described below
commit 8cf552aa3642be473c6a08ce44feceb9cbe396d7
Author: aparnasuresh85 <[email protected]>
AuthorDate: Tue Apr 2 23:47:15 2024 -0400
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.
(cherry picked from commit 5c399dd526e62644e257b42b7667c25cf500356f)
---
solr/CHANGES.txt | 2 +
.../src/java/org/apache/solr/api/V2HttpCall.java | 66 +++++-----------------
.../java/org/apache/solr/servlet/HttpSolrCall.java | 34 +++++++++++
.../solrj/impl/ZkClientClusterStateProvider.java | 17 +++++-
.../client/solrj/impl/ClusterStateProvider.java | 2 +-
5 files changed, 65 insertions(+), 56 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 401765eab4f..fe8a6ed03fc 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -51,6 +51,8 @@ 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)
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 66b81e4c041..6192505d239 100644
--- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
+++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
@@ -36,14 +36,12 @@ import java.util.List;
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 @@ public class V2HttpCall extends HttpSolrCall {
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 @@ public class V2HttpCall extends HttpSolrCall {
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 748aa64c721..47ce5c4ffc0 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -54,6 +54,7 @@ import java.util.Objects;
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,6 +283,8 @@ public class HttpSolrCall {
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);
@@ -346,6 +349,37 @@ public class HttpSolrCall {
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 f9d202f5949..bc56881e86d 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 @@ public class ZkClientClusterStateProvider
@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 e6b7f2097a4..81bb885c38b 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 @@ public interface ClusterStateProvider extends SolrCloseable {
/**
* 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);
