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/03/09 21:23:33 UTC
[GitHub] [solr] madrob opened a new pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
madrob opened a new pull request #736:
URL: https://github.com/apache/solr/pull/736
https://issues.apache.org/jira/browse/SOLR-16091
I don't think this will actually fix the issue seen by the test, but it will make the logging a little bit easier to follow and hopefully make future failures easier to diagnose.
--
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] madrob merged pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
madrob merged pull request #736:
URL: https://github.com/apache/solr/pull/736
--
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] madrob commented on a change in pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r826209848
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -548,16 +473,28 @@ public void m2(SolrQueryRequest req, SolrQueryResponse rsp) {
}
}
+ private Callable<V2Response> getPlugin(String path) {
Review comment:
I added randomization to either forceV2 or not, and manually tested it both ways. I'm not sure what the intent was before, but let's improve test coverage while we're in the area.
--
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] madrob commented on a change in pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r825097245
##########
File path: solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
##########
@@ -225,6 +229,11 @@ public String toString() {
}
}
+ public static <T extends NavigableObject> T assertResponseValues(
+ Callable<T> callable, Map<String, Object> vals) throws Exception {
+ return assertResponseValues(1, callable, vals);
Review comment:
I looked through that and I think it was left over from before SOLR-14749 when we added proper concurrency controls. In this current case, we really shouldn't need to be retrying anymore.
--
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] madrob commented on a change in pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r825127349
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -69,214 +68,168 @@
private Phaser phaser;
@Before
- public void setup() {
+ public void setup() throws Exception {
System.setProperty("enable.packages", "true");
phaser = new Phaser();
+
+ 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 =
+ new V2Request.Builder("/cluster/plugin")
+ .forceV2(true)
+ .POST()
+ .withPayload(singletonMap("add", plugin))
+ .build();
+
+ // 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
+ 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.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);
- }
+ expectError(addPlugin, "path must not have a prefix: collections");
+ assertEquals(1, errors.getCount());
+ }
+
+ 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
+ new V2Request.Builder("/cluster/plugin")
+ .POST()
+ .forceV2(true)
+ .withPayload("{remove : my-random-name}")
+ .build()
+ .process(cluster.getSolrClient());
+
+ version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+ try (ErrorLogMuter errors = ErrorLogMuter.substring("/my-random-prefix/their/plugin")) {
+ expectFail(() -> getPlugin("/my-random-prefix/their/plugin").call());
+ assertEquals(2, errors.getCount());
+ }
Review comment:
I think the two calls before was just a copy-paste bug.
The "two errors" now is because V2Http will log at 146 then log-and-throw at 171. I added some more detail to the test there.
--
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] cpoerschke commented on a change in pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r824944652
##########
File path: solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
##########
@@ -290,6 +299,9 @@ public static NavigableObject assertResponseValues(
Utils.toJSONString(actual));
}
}
+ if (passed) {
+ return rsp;
Review comment:
minor/subjective
```suggestion
break;
```
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -69,214 +68,168 @@
private Phaser phaser;
@Before
- public void setup() {
+ public void setup() throws Exception {
System.setProperty("enable.packages", "true");
phaser = new Phaser();
+
+ 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 =
+ new V2Request.Builder("/cluster/plugin")
+ .forceV2(true)
+ .POST()
+ .withPayload(singletonMap("add", plugin))
+ .build();
+
+ // 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
+ 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.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);
- }
+ expectError(addPlugin, "path must not have a prefix: collections");
+ assertEquals(1, errors.getCount());
+ }
+
+ 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
+ new V2Request.Builder("/cluster/plugin")
+ .POST()
+ .forceV2(true)
+ .withPayload("{remove : my-random-name}")
+ .build()
+ .process(cluster.getSolrClient());
+
+ version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+ try (ErrorLogMuter errors = ErrorLogMuter.substring("/my-random-prefix/their/plugin")) {
+ expectFail(() -> getPlugin("/my-random-prefix/their/plugin").call());
+ assertEquals(2, errors.getCount());
+ }
Review comment:
Two calls before (not sure why) but only one call afterwards, could we have a comment re: why two errors for the one call?
##########
File path: solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
##########
@@ -225,6 +229,11 @@ public String toString() {
}
}
+ public static <T extends NavigableObject> T assertResponseValues(
+ Callable<T> callable, Map<String, Object> vals) throws Exception {
+ return assertResponseValues(1, callable, vals);
Review comment:
Noting that it seems that (at least some) of the callers used `10` repeats before but now the default is `1` here.
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -548,16 +473,28 @@ public void m2(SolrQueryRequest req, SolrQueryResponse rsp) {
}
}
+ private Callable<V2Response> getPlugin(String path) {
Review comment:
Is there a `.forceV2(true)` needed here, sometimes? `V2Request.Builder.forceV2` javadocs say "Only for testing. It's always true otherwise" but that doesn't seem to be true actually?
```suggestion
private Callable<V2Response> getPlugin(String path, bool forceV2) {
```
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -293,129 +246,101 @@ private void expectFail(ThrowingRunnable runnable) throws Exception {
@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]";
Review comment:
```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
[GitHub] [solr] madrob commented on a change in pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r826209013
##########
File path: solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
##########
@@ -225,6 +229,11 @@ public String toString() {
}
}
+ public static <T extends NavigableObject> T assertResponseValues(
+ Callable<T> callable, Map<String, Object> vals) throws Exception {
+ return assertResponseValues(1, callable, vals);
Review comment:
Found a case where we may need multiple attempts, added a comment as to why.
--
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] madrob commented on a change in pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r825097245
##########
File path: solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
##########
@@ -225,6 +229,11 @@ public String toString() {
}
}
+ public static <T extends NavigableObject> T assertResponseValues(
+ Callable<T> callable, Map<String, Object> vals) throws Exception {
+ return assertResponseValues(1, callable, vals);
Review comment:
I looked through that and I think it was left over from before 7ada403 when we added proper concurrency controls. In this current case, we really shouldn't need to be retrying anymore.
--
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] madrob commented on a change in pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r825095467
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -548,16 +473,28 @@ public void m2(SolrQueryRequest req, SolrQueryResponse rsp) {
}
}
+ private Callable<V2Response> getPlugin(String path) {
Review comment:
I think it's always needed to preserve the previous behavior, this was an oversight on my part.
--
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] cpoerschke commented on a change in pull request #736: SOLR-16091 Improve logging for TestContainerPlugin
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r825802770
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -548,16 +473,28 @@ public void m2(SolrQueryRequest req, SolrQueryResponse rsp) {
}
}
+ private Callable<V2Response> getPlugin(String path) {
Review comment:
3/3 Annotated examples of use and non-use of `forceV2(true)` before the change to the getPlugin call.
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -69,353 +69,274 @@
private Phaser phaser;
@Before
- public void setup() {
+ public void setup() throws Exception {
System.setProperty("enable.packages", "true");
phaser = new Phaser();
+
+ 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 =
+ new V2Request.Builder("/cluster/plugin")
+ .forceV2(true)
+ .POST()
+ .withPayload(singletonMap("add", plugin))
+ .build();
+
+ // 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
+ 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.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
+ new V2Request.Builder("/cluster/plugin")
+ .POST()
+ .forceV2(true)
+ .withPayload("{remove : my-random-name}")
+ .build()
+ .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;
+
+ 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(
+ getPlugin("hello/plugin"),
+ Map.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(
+ 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)
Review comment:
1/3 Here's an example of where before the change to the `getPlugin` call the `forceV2(true)` was used.
##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -69,353 +69,274 @@
private Phaser phaser;
@Before
- public void setup() {
+ public void setup() throws Exception {
System.setProperty("enable.packages", "true");
phaser = new Phaser();
+
+ 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 =
+ new V2Request.Builder("/cluster/plugin")
+ .forceV2(true)
+ .POST()
+ .withPayload(singletonMap("add", plugin))
+ .build();
+
+ // 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
+ 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.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
+ new V2Request.Builder("/cluster/plugin")
+ .POST()
+ .forceV2(true)
+ .withPayload("{remove : my-random-name}")
+ .build()
+ .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;
+
+ 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(
+ getPlugin("hello/plugin"),
+ Map.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(
+ 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()
Review comment:
2/3 Here's an example of where before the change to the `getPlugin` call the `forceV2(true)` was _not_ used.
--
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