You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/10/21 19:10:15 UTC

[GitHub] [solr] epugh opened a new pull request, #1105: SOLR-13880: Add test that creates/reloads/deletes collections

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

   https://issues.apache.org/jira/browse/SOLR-13880
   
   
   # Description
   
   Migrate test from https://github.com/apache/lucene-solr/pull/986 to current repo.
   
   
   # Solution
   
   Additional testing.
   


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


[GitHub] [solr] epugh merged pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

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


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


[GitHub] [solr] epugh commented on pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1105:
URL: https://github.com/apache/solr/pull/1105#issuecomment-1287380545

   @risdenk there are a MILLION places where we create raw HttpGet in the tests, I saw that when I tried to migrate things to `SolrClient` or `SolrCloudClient`...      It would be great if we had a general pattern for I want to just do a HTTP thing and look at the results.
   
   As far at not using an API, well, there are two examples, one using the SolrJ, the other with the long string....    


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


[GitHub] [solr] tflobbe commented on a diff in pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

Posted by GitBox <gi...@apache.org>.
tflobbe commented on code in PR #1105:
URL: https://github.com/apache/solr/pull/1105#discussion_r1004687757


##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -86,4 +96,107 @@ public void testReloadedLeaderStateAfterZkSessionLoss() throws Exception {
 
     log.info("testReloadedLeaderStateAfterZkSessionLoss succeeded ... shutting down now!");
   }
+
+  @Repeat(iterations = 5)

Review Comment:
   is it worth having this set to 5? how long does this take? should we have maybe 2 here, and set it higher in nightly? not sure if that's possible



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


[GitHub] [solr] risdenk commented on a diff in pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1105:
URL: https://github.com/apache/solr/pull/1105#discussion_r1002111360


##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -86,4 +96,107 @@ public void testReloadedLeaderStateAfterZkSessionLoss() throws Exception {
 
     log.info("testReloadedLeaderStateAfterZkSessionLoss succeeded ... shutting down now!");
   }
+
+  @Repeat(iterations = 5)
+  public void testCreateReloadDeleteAllNrt() throws Exception {
+    testCreateReloadDelete("testCreateReloadDeleteAllNrt", 3, 0, 0);
+  }
+
+  @Repeat(iterations = 5)
+  public void testCreateReloadDeleteAllTlog() throws Exception {
+    testCreateReloadDelete("testCreateReloadDeleteAllTlog", 0, 3, 0);
+  }
+
+  @Repeat(iterations = 5)
+  public void testCreateReloadDeletePull() throws Exception {
+    testCreateReloadDelete("testCreateReloadDeletePull", 0, 1, 2);
+  }
+
+  private void testCreateReloadDelete(
+      String collectionName, int nrtReplicas, int tlogReplicas, int pullReplicas) throws Exception {
+    int numShards = 3;
+    createCollection(numShards, nrtReplicas, tlogReplicas, pullReplicas, collectionName);
+    boolean reloaded = false;
+    while (true) {
+      waitForState(
+          "Timeout waiting for collection " + collectionName,
+          collectionName,
+          clusterShape(numShards, numShards * (nrtReplicas + tlogReplicas + pullReplicas)));
+      if (reloaded) {
+        break;
+      } else {
+        // reload
+        assertSuccessfulAdminRequest(
+            CollectionAdminRequest.reloadCollection(collectionName)
+                .process(cluster.getSolrClient()));
+        reloaded = true;
+      }
+    }
+    assertSuccessfulAdminRequest(
+        CollectionAdminRequest.deleteCollection(collectionName).process(cluster.getSolrClient()));
+  }
+
+  private void assertSuccessfulAdminRequest(CollectionAdminResponse response) {
+    assertEquals("Unexpected response status", 0, response.getStatus());
+    assertTrue("Unsuccessful response: " + response, response.isSuccess());
+  }
+
+  private void createCollection(
+      int numShards, int nrtReplicas, int tlogReplicas, int pullReplicas, String collectionName)
+      throws SolrServerException, IOException {
+    switch (random().nextInt(3)) {
+      case 0:
+        log.info("Creating collection with SolrJ");
+        // Sometimes use SolrJ
+        assertSuccessfulAdminRequest(
+            CollectionAdminRequest.createCollection(
+                    collectionName, "conf", numShards, nrtReplicas, tlogReplicas, pullReplicas)
+                .process(cluster.getSolrClient()));
+        break;
+      case 1:
+        log.info("Creating collection with V1 API");
+        // Sometimes use v1 API
+        String url =
+            String.format(
+                Locale.ROOT,
+                "%s/admin/collections?action=CREATE&name=%s&collection.configName=%s&numShards=%s&maxShardsPerNode=%s&nrtReplicas=%s&tlogReplicas=%s&pullReplicas=%s",
+                cluster.getRandomJetty(random()).getBaseUrl(),
+                collectionName,
+                "conf",
+                numShards,
+                100, // maxShardsPerNode
+                nrtReplicas,
+                tlogReplicas,
+                pullReplicas);
+        HttpGet createCollectionGet = new HttpGet(url);
+        ((CloudLegacySolrClient) cluster.getSolrClient())

Review Comment:
   Do we need this to be `CloudLegacySolrClient`?



##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -86,4 +96,107 @@ public void testReloadedLeaderStateAfterZkSessionLoss() throws Exception {
 
     log.info("testReloadedLeaderStateAfterZkSessionLoss succeeded ... shutting down now!");
   }
+
+  @Repeat(iterations = 5)
+  public void testCreateReloadDeleteAllNrt() throws Exception {
+    testCreateReloadDelete("testCreateReloadDeleteAllNrt", 3, 0, 0);
+  }
+
+  @Repeat(iterations = 5)
+  public void testCreateReloadDeleteAllTlog() throws Exception {
+    testCreateReloadDelete("testCreateReloadDeleteAllTlog", 0, 3, 0);
+  }
+
+  @Repeat(iterations = 5)
+  public void testCreateReloadDeletePull() throws Exception {
+    testCreateReloadDelete("testCreateReloadDeletePull", 0, 1, 2);
+  }
+
+  private void testCreateReloadDelete(
+      String collectionName, int nrtReplicas, int tlogReplicas, int pullReplicas) throws Exception {
+    int numShards = 3;
+    createCollection(numShards, nrtReplicas, tlogReplicas, pullReplicas, collectionName);
+    boolean reloaded = false;
+    while (true) {
+      waitForState(
+          "Timeout waiting for collection " + collectionName,
+          collectionName,
+          clusterShape(numShards, numShards * (nrtReplicas + tlogReplicas + pullReplicas)));
+      if (reloaded) {
+        break;
+      } else {
+        // reload
+        assertSuccessfulAdminRequest(
+            CollectionAdminRequest.reloadCollection(collectionName)
+                .process(cluster.getSolrClient()));
+        reloaded = true;
+      }
+    }
+    assertSuccessfulAdminRequest(
+        CollectionAdminRequest.deleteCollection(collectionName).process(cluster.getSolrClient()));
+  }
+
+  private void assertSuccessfulAdminRequest(CollectionAdminResponse response) {
+    assertEquals("Unexpected response status", 0, response.getStatus());
+    assertTrue("Unsuccessful response: " + response, response.isSuccess());
+  }
+
+  private void createCollection(
+      int numShards, int nrtReplicas, int tlogReplicas, int pullReplicas, String collectionName)
+      throws SolrServerException, IOException {
+    switch (random().nextInt(3)) {

Review Comment:
   Seems weird these create strings - don't we have admin API wrappers around these? We shouldn't need to build straight httpget/httppost commands...



##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -16,10 +16,20 @@
  */
 package org.apache.solr.cloud.api.collections;
 
+import com.carrotsearch.randomizedtesting.annotations.Repeat;
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.util.Locale;
 import java.util.concurrent.TimeUnit;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.StringEntity;
 import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;

Review Comment:
   Use non legacy client?



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


[GitHub] [solr] epugh commented on pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

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

   @risdenk @tflobbe I was looking at the rather long list of PR's I seem to have open on Solr, and want to get them either committed or closed..    I'm up for merging this one unless either of you has a reason to not?


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


[GitHub] [solr] epugh commented on a diff in pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

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


##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -16,10 +16,20 @@
  */
 package org.apache.solr.cloud.api.collections;
 
+import com.carrotsearch.randomizedtesting.annotations.Repeat;
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.util.Locale;
 import java.util.concurrent.TimeUnit;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.StringEntity;
 import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;

Review Comment:
   turns out we use this to get a `HttpClient` in order to do a direct GET..



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


[GitHub] [solr] risdenk commented on pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1105:
URL: https://github.com/apache/solr/pull/1105#issuecomment-1287395562

   > there are a MILLION places where we create raw HttpGet in the tests
   
   yea more of a we can skip it now but probably has to be fixed at some point :)


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


[GitHub] [solr] epugh commented on pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1105:
URL: https://github.com/apache/solr/pull/1105#issuecomment-1289712518

   @risdenk what are your thoughts on the `CloudLegacySolrClient`?  Did my explanation of why make sense?  Likewise the api call methods, does that make sense?  Or need reworking to have this ready to go... 


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


[GitHub] [solr] epugh commented on a diff in pull request #1105: SOLR-13880: Add test that creates/reloads/deletes collections

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


##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -86,4 +96,107 @@ public void testReloadedLeaderStateAfterZkSessionLoss() throws Exception {
 
     log.info("testReloadedLeaderStateAfterZkSessionLoss succeeded ... shutting down now!");
   }
+
+  @Repeat(iterations = 5)

Review Comment:
   two works for me...    Makign the change.



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