You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/11/03 13:55:08 UTC

[GitHub] [lucene-solr] anshumg commented on a change in pull request #2012: SOLR-14935: Solr can forward request ( remoteQuery ) even if there are local cores present

anshumg commented on a change in pull request #2012:
URL: https://github.com/apache/lucene-solr/pull/2012#discussion_r516219411



##########
File path: solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
##########
@@ -280,30 +280,37 @@ protected void init() throws Exception {
       collectionsList = resolveCollectionListOrAlias(queryParams.get(COLLECTION_PROP, def)); // &collection= takes precedence
 
       if (core == null) {
-        // lookup core from collection, or route away if need to
-        String collectionName = collectionsList.isEmpty() ? null : collectionsList.get(0); // route to 1st
-        //TODO try the other collections if can't find a local replica of the first?   (and do to V2HttpSolrCall)
-
         boolean isPreferLeader = (path.endsWith("/update") || path.contains("/update/"));
 
-        core = getCoreByCollection(collectionName, isPreferLeader); // find a local replica/core for the collection
-        if (core != null) {
-          if (idx > 0) {
-            path = path.substring(idx);
-          }
-        } else {
-          // if we couldn't find it locally, look on other nodes
-          if (idx > 0) {
-            extractRemotePath(collectionName, origCorename);
-            if (action == REMOTEQUERY) {
+        // Let's try finding a local core
+        for (String collectionName: collectionsList) {
+          core = getCoreByCollection(collectionName, isPreferLeader); // find a local replica/core for the collection
+          if (core != null) {
+            if (idx > 0) {
               path = path.substring(idx);
-              return;
             }
+            break;
+          }
+        }
+      }
+
+      // There is no local core so using remoteQuery
+      //TODO try the other collections if can't find a local replica of the first?   (and do to V2HttpSolrCall)
+      if (core == null) {
+        // lookup core from collection, or route away if need to

Review comment:
       This part is going to route away as the lookup already failed locally by this point. (just suggesting the comment fix :) )

##########
File path: solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
##########
@@ -280,30 +280,37 @@ protected void init() throws Exception {
       collectionsList = resolveCollectionListOrAlias(queryParams.get(COLLECTION_PROP, def)); // &collection= takes precedence
 
       if (core == null) {
-        // lookup core from collection, or route away if need to
-        String collectionName = collectionsList.isEmpty() ? null : collectionsList.get(0); // route to 1st
-        //TODO try the other collections if can't find a local replica of the first?   (and do to V2HttpSolrCall)
-
         boolean isPreferLeader = (path.endsWith("/update") || path.contains("/update/"));
 
-        core = getCoreByCollection(collectionName, isPreferLeader); // find a local replica/core for the collection
-        if (core != null) {
-          if (idx > 0) {
-            path = path.substring(idx);
-          }
-        } else {
-          // if we couldn't find it locally, look on other nodes
-          if (idx > 0) {
-            extractRemotePath(collectionName, origCorename);
-            if (action == REMOTEQUERY) {
+        // Let's try finding a local core
+        for (String collectionName: collectionsList) {
+          core = getCoreByCollection(collectionName, isPreferLeader); // find a local replica/core for the collection
+          if (core != null) {
+            if (idx > 0) {
               path = path.substring(idx);
-              return;
             }
+            break;
+          }
+        }
+      }
+
+      // There is no local core so using remoteQuery
+      //TODO try the other collections if can't find a local replica of the first?   (and do to V2HttpSolrCall)
+      if (core == null) {
+        // lookup core from collection, or route away if need to
+        String collectionName = collectionsList.isEmpty() ? null : collectionsList.get(0); // route to 1st
+
+        // if we couldn't find it locally, look on other nodes

Review comment:
       `s/if//` ? We are sure we couldn't find the core locally by this point, right?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org