You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by md...@apache.org on 2022/03/23 15:19:44 UTC

[solr] branch branch_9x updated (8190b50 -> be99dea)

This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a change to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git.


    from 8190b50  SOLR-9529: Make multivalued dates dynamic field use dates field type (#484)
     new 9373b1c  SOLR-16091 Improve test logging (#736)
     new be99dea  SOLR-16091 Wait for all nodes to register plugins in test (#747)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/solr/api/ContainerPluginsRegistry.java  |   2 -
 .../src/java/org/apache/solr/api/V2HttpCall.java   |  19 +-
 .../solr/filestore/TestDistribPackageStore.java    |  34 +-
 .../apache/solr/handler/TestContainerPlugin.java   | 551 +++++++++------------
 4 files changed, 257 insertions(+), 349 deletions(-)

[solr] 01/02: SOLR-16091 Improve test logging (#736)

Posted by md...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git

commit 9373b1c2a679ab9bc574d8719bc6b0e5810aa111
Author: Mike Drob <md...@apache.org>
AuthorDate: Mon Mar 14 21:19:12 2022 -0500

    SOLR-16091 Improve test logging (#736)
    
    Improved logging in TestContainerPlugin and TestDistribPackageStore.
    
    Switch an instance of System.out to using a logger.
    Add ErrorLogMute blocks for expected errors.
    Reduce amount of retries when waiting for state since we have proper expected concurrency hooks in place.
    Randomize whether the test forces V2 APIs
    
    Co-authored-by: Christine Poerschke <cp...@apache.org>
    (cherry picked from commit 6d64c14bf238f4c67b91c757923c5818402d71a9)
---
 .../solr/filestore/TestDistribPackageStore.java    |  34 +-
 .../apache/solr/handler/TestContainerPlugin.java   | 547 +++++++++------------
 2 files changed, 247 insertions(+), 334 deletions(-)

diff --git a/solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java b/solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
index ea99264..0f7bbee 100644
--- a/solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
+++ b/solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
@@ -23,6 +23,7 @@ import static org.hamcrest.CoreMatchers.containsString;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
 import java.nio.ByteBuffer;
 import java.nio.file.Paths;
 import java.util.Collections;
@@ -56,10 +57,13 @@ import org.apache.zookeeper.server.ByteBufferInputStream;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @LogLevel(
     "org.apache.solr.filestore.PackageStoreAPI=DEBUG;org.apache.solr.filestore.DistribPackageStore=DEBUG")
 public class TestDistribPackageStore extends SolrCloudTestCase {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   @Before
   public void setup() {
@@ -225,6 +229,11 @@ public class TestDistribPackageStore extends SolrCloudTestCase {
     }
   }
 
+  public static <T extends NavigableObject> T assertResponseValues(
+      Callable<T> callable, Map<String, Object> vals) throws Exception {
+    return assertResponseValues(1, callable, vals);
+  }
+
   public static NavigableObject assertResponseValues(
       int repeats, SolrClient client, SolrRequest<?> req, Map<String, Object> vals)
       throws Exception {
@@ -245,9 +254,9 @@ public class TestDistribPackageStore extends SolrCloudTestCase {
    * @throws Exception if the callable throws an Exception, or on interrupt between retries
    */
   @SuppressWarnings({"unchecked"})
-  public static NavigableObject assertResponseValues(
-      int repeats, Callable<NavigableObject> callable, Map<String, Object> vals) throws Exception {
-    NavigableObject rsp = null;
+  public static <T extends NavigableObject> T assertResponseValues(
+      int repeats, Callable<T> callable, Map<String, Object> vals) throws Exception {
+    T rsp = null;
 
     for (int i = 0; i < repeats; i++) {
       if (i > 0) {
@@ -290,6 +299,9 @@ public class TestDistribPackageStore extends SolrCloudTestCase {
               Utils.toJSONString(actual));
         }
       }
+      if (passed) {
+        break;
+      }
     }
     return rsp;
   }
@@ -300,19 +312,9 @@ public class TestDistribPackageStore extends SolrCloudTestCase {
     try (HttpSolrClient client = (HttpSolrClient) jetty.newClient()) {
       PackageUtils.uploadKey(
           bytes, path, Paths.get(jetty.getCoreContainer().getSolrHome()), client);
-      Object resp =
-          Utils.executeGET(
-              client.getHttpClient(),
-              jetty.getBaseURLV2().toString() + "/node/files" + path + "?sync=true",
-              null);
-      System.out.println(
-          "sync resp: "
-              + jetty.getBaseURLV2().toString()
-              + "/node/files"
-              + path
-              + "?sync=true"
-              + " ,is: "
-              + resp);
+      String url = jetty.getBaseURLV2() + "/node/files" + path + "?sync=true";
+      Object resp = Utils.executeGET(client.getHttpClient(), url, null);
+      log.info("sync resp: {} was {}", url, resp);
     }
     checkAllNodesForFile(
         cluster,
diff --git a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
index 39b939d..0769613 100644
--- a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
+++ b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
@@ -22,8 +22,8 @@ import static java.util.Collections.singletonMap;
 import static org.apache.solr.client.solrj.SolrRequest.METHOD.GET;
 import static org.apache.solr.filestore.TestDistribPackageStore.readFile;
 import static org.apache.solr.filestore.TestDistribPackageStore.uploadKey;
+import static org.hamcrest.Matchers.containsString;
 
-import com.google.common.collect.ImmutableMap;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
@@ -49,7 +49,6 @@ import org.apache.solr.client.solrj.response.V2Response;
 import org.apache.solr.cloud.ClusterSingleton;
 import org.apache.solr.cloud.MiniSolrCloudCluster;
 import org.apache.solr.cloud.SolrCloudTestCase;
-import org.apache.solr.common.NavigableObject;
 import org.apache.solr.common.annotation.JsonProperty;
 import org.apache.solr.common.util.ReflectMapWriter;
 import org.apache.solr.core.CoreContainer;
@@ -61,6 +60,7 @@ import org.apache.solr.pkg.TestPackages;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.response.SolrQueryResponse;
 import org.apache.solr.security.PermissionNameProvider;
+import org.apache.solr.util.ErrorLogMuter;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -68,354 +68,243 @@ import org.junit.Test;
 public class TestContainerPlugin extends SolrCloudTestCase {
   private Phaser phaser;
 
+  private boolean forceV2;
+
   @Before
-  public void setup() {
+  public void setup() throws Exception {
     System.setProperty("enable.packages", "true");
     phaser = new Phaser();
+    forceV2 = random().nextBoolean();
+
+    int nodes = TEST_NIGHTLY ? 4 : 2;
+    cluster = configureCluster(nodes).withJettyConfig(jetty -> jetty.enableV2(true)).configure();
+    cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry().setPhaser(phaser);
   }
 
   @After
-  public void teardown() {
+  public void teardown() throws Exception {
+    shutdownCluster();
     System.clearProperty("enable.packages");
   }
 
   @Test
   public void testApi() throws Exception {
-    MiniSolrCloudCluster cluster =
-        configureCluster(4).withJettyConfig(jetty -> jetty.enableV2(true)).configure();
-    ContainerPluginsRegistry pluginsRegistry =
-        cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry();
-    pluginsRegistry.setPhaser(phaser);
-
     int version = phaser.getPhase();
 
-    String errPath = "/error/details[0]/errorMessages[0]";
-    try {
-      PluginMeta plugin = new PluginMeta();
+    PluginMeta plugin = new PluginMeta();
+    V2Request addPlugin = postPlugin(singletonMap("add", plugin));
+
+    // test with an invalid class
+    try (ErrorLogMuter errors = ErrorLogMuter.substring("TestContainerPlugin$C2")) {
       plugin.name = "testplugin";
       plugin.klass = C2.class.getName();
-      // test with an invalid class
-      V2Request req =
-          new V2Request.Builder("/cluster/plugin")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", plugin))
-              .build();
-      expectError(req, cluster.getSolrClient(), errPath, "No method with @Command in class");
-
-      // test with a valid class. This should succeed now
-      plugin.klass = C3.class.getName();
-      req.process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      // just check if the plugin is indeed registered
-      V2Request readPluginState =
-          new V2Request.Builder("/cluster/plugin").forceV2(true).GET().build();
-      V2Response rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(C3.class.getName(), rsp._getStr("/plugin/testplugin/class", null));
-
-      // let's test the plugin
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/plugin/my/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/testkey", "testval"));
-
-      // now remove the plugin
-      new V2Request.Builder("/cluster/plugin")
-          .POST()
-          .forceV2(true)
-          .withPayload("{remove : testplugin}")
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      // verify it is removed
-      rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(null, rsp._get("/plugin/testplugin/class", null));
+      expectError(addPlugin, "No method with @Command in class");
+      assertEquals(1, errors.getCount());
+    }
+
+    // test with a valid class. This should succeed now
+    plugin.klass = C3.class.getName();
+    addPlugin.process(cluster.getSolrClient());
 
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // just check if the plugin is indeed registered
+    Callable<V2Response> readPluginState = getPlugin("/cluster/plugin");
+    V2Response rsp = readPluginState.call();
+    assertEquals(C3.class.getName(), rsp._getStr("/plugin/testplugin/class", null));
+
+    // let's test the plugin
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/plugin/my/plugin"), Map.of("/testkey", "testval"));
+
+    // now remove the plugin
+    postPlugin("{remove : testplugin}").process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // verify it is removed
+    rsp = readPluginState.call();
+    assertNull(rsp._get("/plugin/testplugin/class", null));
+
+    try (ErrorLogMuter errors = ErrorLogMuter.substring("TestContainerPlugin$C4")) {
       // test with a class  @EndPoint methods. This also uses a template in the path name
       plugin.klass = C4.class.getName();
       plugin.name = "collections";
       plugin.pathPrefix = "collections";
-      expectError(
-          req, cluster.getSolrClient(), errPath, "path must not have a prefix: collections");
-
-      plugin.name = "my-random-name";
-      plugin.pathPrefix = "my-random-prefix";
-
-      req.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      // let's test the plugin
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/my-random-name/my/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/method.name", "m1"));
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/method.name", "m2"));
-      // now remove the plugin
-      new V2Request.Builder("/cluster/plugin")
-          .POST()
-          .forceV2(true)
-          .withPayload("{remove : my-random-name}")
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      expectFail(
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()));
-      expectFail(
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()));
-
-      // test ClusterSingleton plugin
-      plugin.name = "clusterSingleton";
-      plugin.klass = C6.class.getName();
-      req.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      // just check if the plugin is indeed registered
-      readPluginState = new V2Request.Builder("/cluster/plugin").forceV2(true).GET().build();
-      rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(C6.class.getName(), rsp._getStr("/plugin/clusterSingleton/class", null));
-
-      assertTrue("ccProvided", C6.ccProvided);
-      assertTrue("startCalled", C6.startCalled);
-      assertFalse("stopCalled", C6.stopCalled);
-
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new CC()));
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new CC1()));
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new CC2()));
-
-      CConfig cfg = new CConfig();
-      cfg.boolVal = Boolean.TRUE;
-      cfg.strVal = "Something";
-      cfg.longVal = 1234L;
-      PluginMeta p = new PluginMeta();
-      p.name = "hello";
-      p.klass = CC.class.getName();
-      p.config = cfg;
-
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("add", p))
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("hello/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/config/boolVal", "true", "/config/strVal", "Something", "/config/longVal", "1234"));
-
-      cfg.strVal = "Something else";
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("update", p))
-          .build()
-          .process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("hello/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/config/boolVal", "true", "/config/strVal", cfg.strVal, "/config/longVal", "1234"));
-
-      // kill the Overseer leader
-      for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
-        if (!jetty.getCoreContainer().getZkController().getOverseer().isClosed()) {
-          cluster.stopJettySolrRunner(jetty);
-          cluster.waitForJettyToStop(jetty);
-        }
-      }
-      assertTrue("stopCalled", C6.stopCalled);
-    } finally {
-      cluster.shutdown();
+      expectError(addPlugin, "path must not have a prefix: collections");
+      assertEquals(1, errors.getCount());
     }
-  }
 
-  private void expectFail(ThrowingRunnable runnable) throws Exception {
-    for (int i = 0; i < 20; i++) {
-      try {
-        runnable.run();
-      } catch (Throwable throwable) {
-        return;
+    plugin.name = "my-random-name";
+    plugin.pathPrefix = "my-random-prefix";
+
+    addPlugin.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // let's test the plugin
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/my-random-name/my/plugin"), Map.of("/method.name", "m1"));
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/my-random-prefix/their/plugin"), Map.of("/method.name", "m2"));
+    // now remove the plugin
+    postPlugin("{remove : my-random-name}").process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    try (ErrorLogMuter errors = ErrorLogMuter.substring("/my-random-prefix/their/plugin")) {
+      RemoteExecutionException e =
+          assertThrows(
+              RemoteExecutionException.class,
+              () -> getPlugin("/my-random-prefix/their/plugin").call());
+      assertEquals(404, e.code());
+      assertThat(
+          e.getMetaData().findRecursive("error", "msg").toString(),
+          containsString("no core retrieved"));
+      // V2HttpCall will separately log the path and stack trace, probably could be fixed
+      assertEquals(2, errors.getCount());
+    }
+
+    // test ClusterSingleton plugin
+    plugin.name = "clusterSingleton";
+    plugin.klass = C6.class.getName();
+    addPlugin.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // just check if the plugin is indeed registered
+    rsp = readPluginState.call();
+    assertEquals(C6.class.getName(), rsp._getStr("/plugin/clusterSingleton/class", null));
+
+    assertTrue("ccProvided", C6.ccProvided);
+    assertTrue("startCalled", C6.startCalled);
+    assertFalse("stopCalled", C6.stopCalled);
+
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new CC()));
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new CC1()));
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new CC2()));
+
+    CConfig cfg = new CConfig();
+    cfg.boolVal = Boolean.TRUE;
+    cfg.strVal = "Something";
+    cfg.longVal = 1234L;
+    PluginMeta p = new PluginMeta();
+    p.name = "hello";
+    p.klass = CC.class.getName();
+    p.config = cfg;
+
+    postPlugin(singletonMap("add", p)).process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("hello/plugin"),
+        Map.of(
+            "/config/boolVal", "true", "/config/strVal", "Something", "/config/longVal", "1234"));
+
+    cfg.strVal = "Something else";
+    postPlugin(singletonMap("update", p)).process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("hello/plugin"),
+        Map.of("/config/boolVal", "true", "/config/strVal", cfg.strVal, "/config/longVal", "1234"));
+
+    // kill the Overseer leader
+    for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
+      if (!jetty.getCoreContainer().getZkController().getOverseer().isClosed()) {
+        cluster.stopJettySolrRunner(jetty);
+        cluster.waitForJettyToStop(jetty);
       }
-      Thread.sleep(100);
     }
-    fail("should have failed with an exception");
+    assertTrue("stopCalled", C6.stopCalled);
   }
 
   @Test
   public void testApiFromPackage() throws Exception {
-    MiniSolrCloudCluster cluster =
-        configureCluster(4).withJettyConfig(jetty -> jetty.enableV2(true)).configure();
     String FILE1 = "/myplugin/v1.jar";
     String FILE2 = "/myplugin/v2.jar";
-    ContainerPluginsRegistry pluginsRegistry =
-        cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry();
-    pluginsRegistry.setPhaser(phaser);
 
     int version = phaser.getPhase();
 
-    String errPath = "/error/details[0]/errorMessages[0]";
-    try {
-      byte[] derFile = readFile("cryptokeys/pub_key512.der");
-      uploadKey(derFile, PackageStoreAPI.KEYS_DIR + "/pub_key512.der", cluster);
-      TestPackages.postFileAndWait(
-          cluster,
-          "runtimecode/containerplugin.v.1.jar.bin",
-          FILE1,
-          "pmrmWCDafdNpYle2rueAGnU2J6NYlcAey9mkZYbqh+5RdYo2Ln+llLF9voyRj+DDivK9GV1XdtKvD9rgCxlD7Q==");
-      TestPackages.postFileAndWait(
-          cluster,
-          "runtimecode/containerplugin.v.2.jar.bin",
-          FILE2,
-          "StR3DmqaUSL7qjDOeVEiCqE+ouiZAkW99fsL48F9oWG047o7NGgwwZ36iGgzDC3S2tPaFjRAd9Zg4UK7OZLQzg==");
-
-      // We have two versions of the plugin in 2 different jar files. they are already uploaded to
-      // the package store
-      Package.AddVersion add = new Package.AddVersion();
-      add.version = "1.0";
-      add.pkg = "mypkg";
-      add.files = singletonList(FILE1);
-      V2Request addPkgVersionReq =
-          new V2Request.Builder("/cluster/package")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", add))
-              .build();
-      addPkgVersionReq.process(cluster.getSolrClient());
-
-      waitForAllNodesToSync(
-          cluster,
-          "/cluster/package",
-          Map.of(
-              ":result:packages:mypkg[0]:version",
-              "1.0",
-              ":result:packages:mypkg[0]:files[0]",
-              FILE1));
-
-      // Now lets create a plugin using v1 jar file
-      PluginMeta plugin = new PluginMeta();
-      plugin.name = "myplugin";
-      plugin.klass = "mypkg:org.apache.solr.handler.MyPlugin";
-      plugin.version = add.version;
-      final V2Request req1 =
-          new V2Request.Builder("/cluster/plugin")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", plugin))
-              .build();
-      req1.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      // verify the plugin creation
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/cluster/plugin")
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/plugin/myplugin/class", plugin.klass,
-              "/plugin/myplugin/version", plugin.version));
-      // let's test this now
-      Callable<NavigableObject> invokePlugin =
-          () ->
-              new V2Request.Builder("/plugin/my/path")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient());
-      TestDistribPackageStore.assertResponseValues(
-          10, invokePlugin, ImmutableMap.of("/myplugin.version", "1.0"));
-
-      // now let's upload the jar file for version 2.0 of the plugin
-      add.version = "2.0";
-      add.files = singletonList(FILE2);
-      addPkgVersionReq.process(cluster.getSolrClient());
-
-      // here the plugin version is updated
-      plugin.version = add.version;
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("update", plugin))
-          .build()
-          .process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-
-      // now verify if it is indeed updated
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/cluster/plugin")
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/plugin/myplugin/class", plugin.klass, "/plugin/myplugin/version", "2.0"));
-      // invoke the plugin and test thye output
-      TestDistribPackageStore.assertResponseValues(
-          10, invokePlugin, ImmutableMap.of("/myplugin.version", "2.0"));
-
-      plugin.name = "plugin2";
-      plugin.klass = "mypkg:" + C5.class.getName();
-      plugin.version = "2.0";
-      req1.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
-      assertNotNull(C5.classData);
-      assertEquals(1452, C5.classData.limit());
-    } finally {
-      cluster.shutdown();
-    }
+    byte[] derFile = readFile("cryptokeys/pub_key512.der");
+    uploadKey(derFile, PackageStoreAPI.KEYS_DIR + "/pub_key512.der", cluster);
+    TestPackages.postFileAndWait(
+        cluster,
+        "runtimecode/containerplugin.v.1.jar.bin",
+        FILE1,
+        "pmrmWCDafdNpYle2rueAGnU2J6NYlcAey9mkZYbqh+5RdYo2Ln+llLF9voyRj+DDivK9GV1XdtKvD9rgCxlD7Q==");
+    TestPackages.postFileAndWait(
+        cluster,
+        "runtimecode/containerplugin.v.2.jar.bin",
+        FILE2,
+        "StR3DmqaUSL7qjDOeVEiCqE+ouiZAkW99fsL48F9oWG047o7NGgwwZ36iGgzDC3S2tPaFjRAd9Zg4UK7OZLQzg==");
+
+    // We have two versions of the plugin in 2 different jar files. they are already uploaded to
+    // the package store
+    Package.AddVersion add = new Package.AddVersion();
+    add.version = "1.0";
+    add.pkg = "mypkg";
+    add.files = singletonList(FILE1);
+    V2Request addPkgVersionReq =
+        new V2Request.Builder("/cluster/package")
+            .forceV2(forceV2)
+            .POST()
+            .withPayload(singletonMap("add", add))
+            .build();
+    addPkgVersionReq.process(cluster.getSolrClient());
+
+    waitForAllNodesToSync(
+        cluster,
+        "/cluster/package",
+        Map.of(
+            ":result:packages:mypkg[0]:version",
+            "1.0",
+            ":result:packages:mypkg[0]:files[0]",
+            FILE1));
+
+    // Now lets create a plugin using v1 jar file
+    PluginMeta plugin = new PluginMeta();
+    plugin.name = "myplugin";
+    plugin.klass = "mypkg:org.apache.solr.handler.MyPlugin";
+    plugin.version = add.version;
+    final V2Request addPluginReq = postPlugin(singletonMap("add", plugin));
+    addPluginReq.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // verify the plugin creation
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/cluster/plugin"),
+        Map.of(
+            "/plugin/myplugin/class", plugin.klass,
+            "/plugin/myplugin/version", plugin.version));
+    // let's test this now
+    Callable<V2Response> invokePlugin = getPlugin("/plugin/my/path");
+    TestDistribPackageStore.assertResponseValues(invokePlugin, Map.of("/myplugin.version", "1.0"));
+
+    // now let's upload the jar file for version 2.0 of the plugin
+    add.version = "2.0";
+    add.files = singletonList(FILE2);
+    addPkgVersionReq.process(cluster.getSolrClient());
+
+    // here the plugin version is updated
+    plugin.version = add.version;
+    postPlugin(singletonMap("update", plugin)).process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // now verify if it is indeed updated
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/cluster/plugin"),
+        Map.of("/plugin/myplugin/class", plugin.klass, "/plugin/myplugin/version", "2.0"));
+    // invoke the plugin and test thye output
+    TestDistribPackageStore.assertResponseValues(invokePlugin, Map.of("/myplugin.version", "2.0"));
+
+    plugin.name = "plugin2";
+    plugin.klass = "mypkg:" + C5.class.getName();
+    plugin.version = "2.0";
+    addPluginReq.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+    assertNotNull(C5.classData);
+    assertEquals(1452, C5.classData.limit());
   }
 
   public static class CC1 extends CC {}
@@ -548,16 +437,38 @@ public class TestContainerPlugin extends SolrCloudTestCase {
     }
   }
 
+  private Callable<V2Response> getPlugin(String path) {
+    V2Request req = new V2Request.Builder(path).forceV2(forceV2).GET().build();
+    return () -> req.process(cluster.getSolrClient());
+  }
+
+  private V2Request postPlugin(Object payload) {
+    return new V2Request.Builder("/cluster/plugin")
+        .forceV2(forceV2)
+        .POST()
+        .withPayload(payload)
+        .build();
+  }
+
   public static void waitForAllNodesToSync(
       MiniSolrCloudCluster cluster, String path, Map<String, Object> expected) throws Exception {
     for (JettySolrRunner jettySolrRunner : cluster.getJettySolrRunners()) {
       String baseUrl = jettySolrRunner.getBaseUrl().toString().replace("/solr", "/api");
       String url = baseUrl + path + "?wt=javabin";
+      // Allow multiple retries here because we need multiple nodes to update
+      // and our single phaser only ensures that one of them has reached expected state
       TestDistribPackageStore.assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
     }
   }
 
-  private void expectError(V2Request req, SolrClient client, String errPath, String expectErrorMsg)
+  private void expectError(V2Request req, String expectErrorMsg)
+      throws IOException, SolrServerException {
+    String errPath = "/error/details[0]/errorMessages[0]";
+    expectError(req, cluster.getSolrClient(), errPath, expectErrorMsg);
+  }
+
+  private static void expectError(
+      V2Request req, SolrClient client, String errPath, String expectErrorMsg)
       throws IOException, SolrServerException {
     RemoteExecutionException e =
         expectThrows(RemoteExecutionException.class, () -> req.process(client));

[solr] 02/02: SOLR-16091 Wait for all nodes to register plugins in test (#747)

Posted by md...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git

commit be99dea69a5e72b3eb6e52b54e93b4cc03932b64
Author: Mike Drob <md...@apache.org>
AuthorDate: Fri Mar 18 09:19:13 2022 -0500

    SOLR-16091 Wait for all nodes to register plugins in test (#747)
    
    (cherry picked from commit c10fac98cb1207786cfc5f651456207edbf25c00)
---
 .../org/apache/solr/api/ContainerPluginsRegistry.java |  2 --
 .../core/src/java/org/apache/solr/api/V2HttpCall.java | 19 ++++++++++---------
 .../org/apache/solr/handler/TestContainerPlugin.java  | 18 +++++++-----------
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java b/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
index 74affd6..f8d2e64 100644
--- a/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
+++ b/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
@@ -87,8 +87,6 @@ public class ContainerPluginsRegistry implements ClusterPropertiesListener, MapW
     refresh();
     Phaser localPhaser = phaser; // volatile read
     if (localPhaser != null) {
-      assert localPhaser.getRegisteredParties() == 1;
-      // we should be the only ones registered, so this will advance phase each time
       localPhaser.arrive();
     }
     return false;
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 a463ce3..b5b27df 100644
--- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
+++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
@@ -95,8 +95,8 @@ public class V2HttpCall extends HttpSolrCall {
       }
 
       boolean isCompositeApi = false;
+      api = getApiInfo(cores.getRequestHandlers(), path, req.getMethod(), fullPath, parts);
       if (knownPrefixes.contains(prefix)) {
-        api = getApiInfo(cores.getRequestHandlers(), path, req.getMethod(), fullPath, parts);
         if (api != null) {
           isCompositeApi = api instanceof CompositeApi;
           if (!isCompositeApi) {
@@ -104,6 +104,14 @@ public class V2HttpCall extends HttpSolrCall {
             return;
           }
         }
+      } else { // custom plugin
+        if (api != null) {
+          initAdminRequest(path);
+          return;
+        }
+        assert core == null;
+        throw new SolrException(
+            SolrException.ErrorCode.NOT_FOUND, "Could not load plugin at " + path);
       }
 
       if ("c".equals(prefix) || "collections".equals(prefix)) {
@@ -134,13 +142,6 @@ public class V2HttpCall extends HttpSolrCall {
       } else if ("cores".equals(prefix)) {
         origCorename = pathSegments.get(1);
         core = cores.getCore(origCorename);
-      } else {
-        api = getApiInfo(cores.getRequestHandlers(), path, req.getMethod(), fullPath, parts);
-        if (api != null) {
-          // custom plugin
-          initAdminRequest(path);
-          return;
-        }
       }
       if (core == null) {
         log.error(">> path: '{}'", path);
@@ -150,7 +151,7 @@ public class V2HttpCall extends HttpSolrCall {
         } else {
           throw new SolrException(
               SolrException.ErrorCode.NOT_FOUND,
-              "no core retrieved for core name:  " + origCorename + ". Path : " + path);
+              "no core retrieved for core name: " + origCorename + ". Path: " + path);
         }
       }
 
diff --git a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
index 0769613..1c7b07b 100644
--- a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
+++ b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
@@ -47,7 +47,6 @@ import org.apache.solr.client.solrj.request.beans.Package;
 import org.apache.solr.client.solrj.request.beans.PluginMeta;
 import org.apache.solr.client.solrj.response.V2Response;
 import org.apache.solr.cloud.ClusterSingleton;
-import org.apache.solr.cloud.MiniSolrCloudCluster;
 import org.apache.solr.cloud.SolrCloudTestCase;
 import org.apache.solr.common.annotation.JsonProperty;
 import org.apache.solr.common.util.ReflectMapWriter;
@@ -78,7 +77,9 @@ public class TestContainerPlugin extends SolrCloudTestCase {
 
     int nodes = TEST_NIGHTLY ? 4 : 2;
     cluster = configureCluster(nodes).withJettyConfig(jetty -> jetty.enableV2(true)).configure();
-    cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry().setPhaser(phaser);
+    for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
+      jetty.getCoreContainer().getContainerPluginsRegistry().setPhaser(phaser);
+    }
   }
 
   @After
@@ -160,9 +161,8 @@ public class TestContainerPlugin extends SolrCloudTestCase {
       assertEquals(404, e.code());
       assertThat(
           e.getMetaData().findRecursive("error", "msg").toString(),
-          containsString("no core retrieved"));
-      // V2HttpCall will separately log the path and stack trace, probably could be fixed
-      assertEquals(2, errors.getCount());
+          containsString("Could not load plugin at"));
+      assertEquals(1, errors.getCount());
     }
 
     // test ClusterSingleton plugin
@@ -254,7 +254,6 @@ public class TestContainerPlugin extends SolrCloudTestCase {
     addPkgVersionReq.process(cluster.getSolrClient());
 
     waitForAllNodesToSync(
-        cluster,
         "/cluster/package",
         Map.of(
             ":result:packages:mypkg[0]:version",
@@ -450,14 +449,11 @@ public class TestContainerPlugin extends SolrCloudTestCase {
         .build();
   }
 
-  public static void waitForAllNodesToSync(
-      MiniSolrCloudCluster cluster, String path, Map<String, Object> expected) throws Exception {
+  public void waitForAllNodesToSync(String path, Map<String, Object> expected) throws Exception {
     for (JettySolrRunner jettySolrRunner : cluster.getJettySolrRunners()) {
       String baseUrl = jettySolrRunner.getBaseUrl().toString().replace("/solr", "/api");
       String url = baseUrl + path + "?wt=javabin";
-      // Allow multiple retries here because we need multiple nodes to update
-      // and our single phaser only ensures that one of them has reached expected state
-      TestDistribPackageStore.assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
+      TestDistribPackageStore.assertResponseValues(1, new Fetcher(url, jettySolrRunner), expected);
     }
   }