You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "aparnasuresh85 (via GitHub)" <gi...@apache.org> on 2024/03/21 14:22:12 UTC

[PR] CloudSolrClient should not throw "Collection not found" with an outda… [solr]

aparnasuresh85 opened a new pull request, #2363:
URL: https://github.com/apache/solr/pull/2363

   …ted cluster state
   
   https://issues.apache.org/jira/browse/SOLR-17153
   
   <!--
   _(If you are a project committer then you may remove some/all of the following template.)_
   
   Before creating a pull request, please file an issue in the ASF Jira system for Solr:
   
   * https://issues.apache.org/jira/projects/SOLR
   
   For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA. 
   To create a Jira issue, you will need to create an account there first.
   
   The title of the PR should reference the Jira issue number in the form:
   
   * SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState
   
   SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
   
   Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
   
   
   # Description
   
   At present, when CloudSolrClient is instructed to send a request to a collection it deems non-existent due to its outdated local ClusterState view, it fails locally, which is undesired. This failure should be addressed. Moreover, many SolrCloud tests should remove their waitForState calls following collection creation. Matters concerning other stale states are not addressed here.
   
   Similar changes will be required on the server side to handle "Collection Not Found" errors resulting from outdated cluster state.
   
   # Solution
   
   If neither CloudSolrClient nor HttpSolrCall (server) can locate a collection, trigger a forced fetch of the collection from ZooKeeper and update the local cluster state accordingly.
   
   # Tests
   
   Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
   Change existing tests to eliminate **waitForState()** and **waitForActiveCollection()** calls that were previously present after collection creation. These calls are unnecessary due to the enhancements introduced in this PR. All modified tests have been thoroughly verified to ensure their successful execution.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [X] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability.
   - [X] I have created a Jira issue and added the issue ID to my pull request title.
   - [X] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [X] I have developed this patch against the `main` branch.
   - [X] I have run `./gradlew check`.
   - [] I have added tests for my changes.
   - [] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2556137265

   @markrmiller , thinking back to your feedback, I have an idea of a solution in several steps:
   (A)  In https://issues.apache.org/jira/browse/SOLR-17601 I propose using HTTP response headers to inform the client of the state version of a collection that the client is interacting with.  Imagine if collection admin commands participated in this (not just handlers on the core) like create collection.  Thus when CREATE returns, the header indicates the version of the final created state.   Let's also assume waitForState=true[1].
   (B) Even though the CloudSolrClient's cache[2] wouldn't yet have a DocCollection, let's imagine an enhancement to this cache in which it can hold a cache entry without a DocCollection but with a version to indicate we now want this version (or greater).  At least for HTTP ClusterStateProvider, it will fetch the CLUSTERSTATE from Solr via HTTP.  What's new -- imagine it adds the \_stateVer\_ parameter.
   (C) HttpSolrCall will see the \_stateVer\_ param.  If it's DocCollection isn't as high as this version, it forces an update.  I think this part works as-is but perhaps some work may be needed.
   
   [1] why the heck is waitForState not true by default?
   [2] the CSC's cache should move to ClusterStateProvider -- https://issues.apache.org/jira/browse/SOLR-17604


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2013499876

   > I'm glad to see the logic that was in V2HttpCall made more common (move to base class) as it should not have been specific to V2. Perhaps my oversight 6 years ago.
   > 
   > Updating ZkClientClusterStateProvider is only one of the two ClusterStateProviders. I suspect the other Http one can be tested to show that this problem existed (and still exists now!) whereas probably too hard to bother with ZkClientClusterStateProvider; unclear how to trigger a pause in ZK watching without being too invasive.
   > 
   > Assuming we address this for both providers, the javadocs for this method ought to communicate the guarantee on not returning null if the collection exists.
   
   The reason I did not make the logic consistent across both providers is because BaseHttpClusterStateProvider, where the logic for getState() is implemented for the http version, checks all livenodes for presence of the collection/collectionRef via a SolrQuery. I felt that was comprehensive enough.
   
   I can update the javadoc for the method as per your suggestion.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "markrmiller (via GitHub)" <gi...@apache.org>.
markrmiller commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2556330775

   Yeah, that’s how I would do it. For any command that alters the state, return the version of the modified state and have the client make sure that’s at least the version it’s using for the next command. The Cloud client does *something* like this for some case(s), (if I remember right, as something that kicks in on retries), but I don’t recall it properly covering this stuff. 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2030147402

   Maybe in this PR or another we'll add a test like you (Mark) describe to ensure that CloudSolrClient directs requests to Solr in all circumstances (giving up on smart routing to a core if this can't be determined) so long as we have confirmed the collection in fact exists (which again requires talking to the backend).  My concern is that, as we saw here before the changes, CloudSolrClient would yield an error prematurely.
   
   Hmmm; I wonder what the state.json looks like at each incremental step of a trivial one-shard 2-replica collection.  Something to investigate.  If it's a bit incremental; it'd be cool if Solr might be improved to get the collection to exist with at least a core in one-shot, thus eliminating the possibility a Solr node having a collection's state that is so partial that it can't even route a request to it.
   
   It'd be nice to remove so many waitForState's but maybe we can't in good conscience.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2363:
URL: https://github.com/apache/solr/pull/2363#discussion_r1535589661


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java:
##########
@@ -52,8 +52,7 @@ static ClusterStateProvider newZkClusterStateProvider(
 
   /**
    * 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

Review Comment:
   ```suggestion
      * @return the collection state, or null only if the collection is certain not to exist
   ```



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2363:
URL: https://github.com/apache/solr/pull/2363#discussion_r1534052101


##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -345,6 +348,41 @@ 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();
+    Supplier<DocCollection> logic =
+        () -> {
+          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));

Review Comment:
   again; the choice to do this for the first collection seems repeated here (see earlier `collectionsList.get(0)`).  Would be clearer to do this once and have a var like "collectionName".



##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -345,6 +348,41 @@ 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();
+    Supplier<DocCollection> logic =
+        () -> {
+          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?

Review Comment:
   I know you are just moving code but these 3 lines seem like they don't belong inside the supplier; should be done before hand.  Won't need to be repeated.



##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java:
##########
@@ -144,11 +144,22 @@ public static ClusterState createFromJsonSupportingLegacyConfigName(
   @Override
   public ClusterState.CollectionRef getState(String collection) {
     ClusterState clusterState = getZkStateReader().getClusterState();
-    if (clusterState != null) {
-      return clusterState.getCollectionRef(collection);
-    } else {
+    if (clusterState == null) {

Review Comment:
   When would this happen?  If it's a startup/initialization scenario, maybe getClusterState() should wait until it exists?  Okay to defer this question to another PR as this concern pre-existed albeit maybe it could be another reason why we aren't resolving the collection.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2363:
URL: https://github.com/apache/solr/pull/2363#discussion_r1536097875


##########
solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java:
##########
@@ -99,17 +99,6 @@ public void testProperties() throws Exception {
     CollectionAdminRequest.createCollection("collection2meta", "conf", 1, 1)
         .process(cluster.getSolrClient());
 
-    cluster.waitForActiveCollection("collection1meta", 2, 2);
-    cluster.waitForActiveCollection("collection2meta", 1, 1);
-
-    waitForState(
-        "Expected collection1 to be created with 2 shards and 1 replica",
-        "collection1meta",
-        clusterShape(2, 2));
-    waitForState(
-        "Expected collection2 to be created with 1 shard and 1 replica",
-        "collection2meta",
-        clusterShape(1, 1));

Review Comment:
   The point of this issue/PR is to be able to use a CloudSolrClient (such as cluster.getSolrClient) after collection creation to make requests to the collection (index/query) without explicitly waiting for its existence.  Nothing more.  This will enable *some* tests to remove waitForState calls that follow collection creation as needless.  Maybe a comment would be nice on waitForState to elaborate on when it might *not* be needed.
   
   Here where I comment, I don't see that we can remove these calls.  If you've thought about this one and think otherwise, let me know.  At this time I don't plan to review the remainder of the tests; you may need to revert some.  And to reduce test flapping risk, I would prefer that these waitForState removal changes are decoupled to another PR that is merged separately.  We might intentionally be delay the 9x backport to make changes here more apparent.  This would also be good for code archeology to see the substance of the change without distraction of touching 30+ other files.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2015349982

   > > The reason I did not make the logic consistent across both providers is because BaseHttpClusterStateProvider, where the logic for getState() is implemented for the http version, checks all livenodes for presence of the collection/collectionRef via a SolrQuery. I felt that was comprehensive enough.
   > 
   > OMG I didn't look; so glad to see the correct implementation there already :-)
   > 
   > Please add a CHANGES.txt for the next 9.x version under Improvements. I suppose this may be ready to merge.
   > 
   > In a separate PR, I'd like to see most waitForState calls that follow collection creation to be removed.
   
   I pushed a commit to exclude waitForState() calls after collection creation. Will do another pass to identify other hidden calls.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2557406048

   IMO, ideally, HttpSolrCall would also use the proposed/enhanced ClusterStateProvider (containing the cache & means to fetch an updated state) to have similar cache semantics available on the backend.  In some sense, it's a new re-imagined ClusterState.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "markrmiller (via GitHub)" <gi...@apache.org>.
markrmiller commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2017601379

   I'd be careful removing all the waits in tests without some pretty thorough testing in some slow environments - for one, it's a bandaid rather than a fix, there is still a race, it's just harder to hit, and for two, the bug does not just apply to the collection level. Collections are created piecemeal, and the collection can exist but, for example, a necessary shard may not yet.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2019808827

   I was aware we may just be moving to a new problem, albeit a situation that is even more rare than what we have today.  @markrmiller can you recommend a specific test or a way to make a test that has an incomplete DocCollection and tries to use it client side to route?  As I write that; maybe it's not too hard to create a cluster state provider in a SolrJ test that has the state we want.  The behavior we want to see client side is that it gets routed to Solr; don't give up client-side.  In practice this gives even more time for the server to get the subsequent request and for this DocCollection to become complete.  But if it's incomplete, I suspect an error.  I've not seen this happen in practice, at least to my knowledge.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2014026536

   > I'm glad to see the logic that was in V2HttpCall made more common (move to base class) as it should not have been specific to V2. Perhaps my oversight 6 years ago.
   > 
   > Updating ZkClientClusterStateProvider is only one of the two ClusterStateProviders. I suspect the other Http one can be tested to show that this problem existed (and still exists now!) whereas probably too hard to bother with ZkClientClusterStateProvider; unclear how to trigger a pause in ZK watching without being too invasive.
   > 
   > Assuming we address this for both providers, the javadocs for this method ought to communicate the guarantee on not returning null if the collection exists.
   
   
   
   I also noticed javadoc for ClusterStateProvider.getState() does mention null means collection does not exit. I refined the wording a bit.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "markrmiller (via GitHub)" <gi...@apache.org>.
markrmiller commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2019851574

   Not off hand, but you can likely just create a collection, create a DocCollection with the state you want, convert it to json with the code that does that, and then overwrite cluster state straight to ZK.
   
   But my point is, at least when I glanced at the change, it looked like it retried if the collection wasn't there. If that is the case, even if that worked 100%,it would only cover a fraction of the fails the waitFor calls handle in tests. They wait for every shard and every replica that a collection api call will create (it's not just collection create calls).
   
   Without ensuring every shard and replica is there, rare, hard to debug timing fails will return (less rare on CI runs or in resource constrained envs), as a test will often do something that expects all parts of what they just said to create to exist once that call has returned. 
   
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley merged PR #2363:
URL: https://github.com/apache/solr/pull/2363


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2363:
URL: https://github.com/apache/solr/pull/2363#discussion_r1536189971


##########
solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java:
##########
@@ -99,17 +99,6 @@ public void testProperties() throws Exception {
     CollectionAdminRequest.createCollection("collection2meta", "conf", 1, 1)
         .process(cluster.getSolrClient());
 
-    cluster.waitForActiveCollection("collection1meta", 2, 2);
-    cluster.waitForActiveCollection("collection2meta", 1, 1);
-
-    waitForState(
-        "Expected collection1 to be created with 2 shards and 1 replica",
-        "collection1meta",
-        clusterShape(2, 2));
-    waitForState(
-        "Expected collection2 to be created with 1 shard and 1 replica",
-        "collection2meta",
-        clusterShape(1, 1));

Review Comment:
   Got it and I agree - will revert the last commit.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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


Re: [PR] SOLR-17153: CloudSolrClient should not throw "Collection not found" with an out-dated ClusterState [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2363:
URL: https://github.com/apache/solr/pull/2363#issuecomment-2016364187

   Proposed CHANGES.txt under 9.6 Bug Fix:
   > CloudSolrClient and Solr could in rare circumstances report that a collection does not exist right after it was actually just created (an eventually consistent race issue).  Now it double-checks before yielding such an error.
   
   It's tempting to call it an improvement but this ought to have been an explicit guarantee all along for such a trivial create-then-index (as seen in tests we have on a high scale cluster).
   
   I plan to merge late Tuesday, or maybe sooner if I get another +1.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


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