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/07/03 17:49:36 UTC

[GitHub] [lucene-solr] chatman opened a new pull request #1644: SOLR-14623: Minor Improvements to SolrCloud tests

chatman opened a new pull request #1644:
URL: https://github.com/apache/lucene-solr/pull/1644


   Here are some of the changes herein:
   1. MiniSolrCloudCluster: Changing the polling to waitForState (based on watch/latch)
   2. Improve waiting for Jetty stop, wait until node is not in liveNodes (JettySolrRunner)
   3. TestCloudConsistency: Explicitly pick the jetty hosting shard1's leader
   4. TestWaitForStateWithJettyShutdowns: Wait until replica becomes active
   5. TestCollectionsAPIViaSolrCloudCluster: Don't wait for the collection to be deleted fully at the end
   6. TestRecovery: Unset the UpdateLog hooks
   


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


[GitHub] [lucene-solr] tflobbe commented on a change in pull request #1644: SOLR-14623: Minor Improvements to SolrCloud tests

Posted by GitBox <gi...@apache.org>.
tflobbe commented on a change in pull request #1644:
URL: https://github.com/apache/lucene-solr/pull/1644#discussion_r449712037



##########
File path: solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java
##########
@@ -207,18 +208,38 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader,
    * Leader should be on node - 0
    */
   private void addDocWhenOtherReplicasAreNetworkPartitioned(String collection, Replica leader, int docId) throws Exception {
-    for (int i = 0; i < 3; i++) {
-      proxies.get(cluster.getJettySolrRunner(i)).close();
+    DocCollection col = cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(collection);
+    Replica shard1Leader = col.getLeader("shard1");
+    String baseUrl = shard1Leader.getBaseUrl();
+    JettySolrRunner j1 = null; // This will be the jetty hosting shard1's leader
+    for (JettySolrRunner j : cluster.getJettySolrRunners()) {
+      System.out.println("cmp:" + j.getProxyBaseUrl() + " " + baseUrl);
+      if (j.getProxyBaseUrl().toString().equals(baseUrl)) {
+        j1 = j;
+        break;
+      }
+    }
+
+    assertNotNull(baseUrl, j1);

Review comment:
       Maybe you can improve this message? A failure would look confusing

##########
File path: solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java
##########
@@ -207,18 +208,38 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader,
    * Leader should be on node - 0
    */
   private void addDocWhenOtherReplicasAreNetworkPartitioned(String collection, Replica leader, int docId) throws Exception {
-    for (int i = 0; i < 3; i++) {
-      proxies.get(cluster.getJettySolrRunner(i)).close();
+    DocCollection col = cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(collection);
+    Replica shard1Leader = col.getLeader("shard1");
+    String baseUrl = shard1Leader.getBaseUrl();
+    JettySolrRunner j1 = null; // This will be the jetty hosting shard1's leader
+    for (JettySolrRunner j : cluster.getJettySolrRunners()) {
+      System.out.println("cmp:" + j.getProxyBaseUrl() + " " + baseUrl);
+      if (j.getProxyBaseUrl().toString().equals(baseUrl)) {
+        j1 = j;
+        break;
+      }
+    }
+
+    assertNotNull(baseUrl, j1);
+
+    for (JettySolrRunner j : cluster.getJettySolrRunners()) {
+      if (j != j1) {
+        proxies.get(j).close();
+      }
     }
-    addDoc(collection, docId, cluster.getJettySolrRunner(0));
-    JettySolrRunner j1 = cluster.getJettySolrRunner(0);
+
+    addDoc(collection, docId, j1);
+
     j1.stop();
     cluster.waitForJettyToStop(j1);
-    for (int i = 1; i < 3; i++) {
-      proxies.get(cluster.getJettySolrRunner(i)).reopen();
+    for (JettySolrRunner j : cluster.getJettySolrRunners()) {
+      if (j != j1) {
+        proxies.get(j).reopen();
+      }
     }
     waitForState("Timeout waiting for leader goes DOWN", collection, (liveNodes, collectionState)
-        -> collectionState.getReplica(leader.getName()).getState() == Replica.State.DOWN);
+        ->  collectionState.getReplica(leader.getName()).getState() == Replica.State.DOWN);
+    Thread.sleep(1000);

Review comment:
       Is this sleep here to wait for the stop? Why is it now needed?

##########
File path: solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java
##########
@@ -207,18 +208,38 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader,
    * Leader should be on node - 0
    */
   private void addDocWhenOtherReplicasAreNetworkPartitioned(String collection, Replica leader, int docId) throws Exception {
-    for (int i = 0; i < 3; i++) {
-      proxies.get(cluster.getJettySolrRunner(i)).close();
+    DocCollection col = cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(collection);
+    Replica shard1Leader = col.getLeader("shard1");
+    String baseUrl = shard1Leader.getBaseUrl();
+    JettySolrRunner j1 = null; // This will be the jetty hosting shard1's leader
+    for (JettySolrRunner j : cluster.getJettySolrRunners()) {
+      System.out.println("cmp:" + j.getProxyBaseUrl() + " " + baseUrl);

Review comment:
       Is this intentionally left here?

##########
File path: solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java
##########
@@ -207,18 +208,38 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader,
    * Leader should be on node - 0

Review comment:
       I guess this is no longer true?

##########
File path: solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java
##########
@@ -207,18 +208,38 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader,
    * Leader should be on node - 0
    */
   private void addDocWhenOtherReplicasAreNetworkPartitioned(String collection, Replica leader, int docId) throws Exception {
-    for (int i = 0; i < 3; i++) {
-      proxies.get(cluster.getJettySolrRunner(i)).close();
+    DocCollection col = cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(collection);
+    Replica shard1Leader = col.getLeader("shard1");
+    String baseUrl = shard1Leader.getBaseUrl();
+    JettySolrRunner j1 = null; // This will be the jetty hosting shard1's leader

Review comment:
       Maybe `s/j1/leaderJetty/` or something?

##########
File path: solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java
##########
@@ -207,18 +208,38 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader,
    * Leader should be on node - 0
    */
   private void addDocWhenOtherReplicasAreNetworkPartitioned(String collection, Replica leader, int docId) throws Exception {
-    for (int i = 0; i < 3; i++) {
-      proxies.get(cluster.getJettySolrRunner(i)).close();
+    DocCollection col = cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(collection);
+    Replica shard1Leader = col.getLeader("shard1");
+    String baseUrl = shard1Leader.getBaseUrl();
+    JettySolrRunner j1 = null; // This will be the jetty hosting shard1's leader
+    for (JettySolrRunner j : cluster.getJettySolrRunners()) {
+      System.out.println("cmp:" + j.getProxyBaseUrl() + " " + baseUrl);
+      if (j.getProxyBaseUrl().toString().equals(baseUrl)) {
+        j1 = j;
+        break;
+      }
+    }
+
+    assertNotNull(baseUrl, j1);
+
+    for (JettySolrRunner j : cluster.getJettySolrRunners()) {
+      if (j != j1) {
+        proxies.get(j).close();
+      }
     }
-    addDoc(collection, docId, cluster.getJettySolrRunner(0));
-    JettySolrRunner j1 = cluster.getJettySolrRunner(0);
+
+    addDoc(collection, docId, j1);
+
     j1.stop();
     cluster.waitForJettyToStop(j1);
-    for (int i = 1; i < 3; i++) {
-      proxies.get(cluster.getJettySolrRunner(i)).reopen();
+    for (JettySolrRunner j : cluster.getJettySolrRunners()) {
+      if (j != j1) {
+        proxies.get(j).reopen();
+      }
     }
     waitForState("Timeout waiting for leader goes DOWN", collection, (liveNodes, collectionState)
-        -> collectionState.getReplica(leader.getName()).getState() == Replica.State.DOWN);
+        ->  collectionState.getReplica(leader.getName()).getState() == Replica.State.DOWN);
+    Thread.sleep(1000);
 
     // the meat of the test -- wait to see if a different replica become a leader
     // the correct behavior is that this should time out, if it succeeds we have a problem...

Review comment:
       Suggestion, if the expected behavior is the timeout here (no leader elected), WDYT about waiting for less time in normal tests (1second maybe?) and only doing 10 seconds on nightly?




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


[GitHub] [lucene-solr] chatman commented on pull request #1644: SOLR-14623: Minor Improvements to SolrCloud tests

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1644:
URL: https://github.com/apache/lucene-solr/pull/1644#issuecomment-657855444


   Closing this, all of these improvements are covered in SOLR-14636.


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


[GitHub] [lucene-solr] chatman closed pull request #1644: SOLR-14623: Minor Improvements to SolrCloud tests

Posted by GitBox <gi...@apache.org>.
chatman closed pull request #1644:
URL: https://github.com/apache/lucene-solr/pull/1644


   


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