SOLR-14749: Make sure the plugin config is reloaded on Overseer.

solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java

@@ -26,11 +26,13 @@ import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.*;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.Phaser;
 import java.util.function.Supplier;
 
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.MapperFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.lucene.util.ResourceLoaderAware;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.request.beans.PluginMeta;
@@ -79,12 +81,30 @@ public class ContainerPluginsRegistry implements ClusterPropertiesListener, MapW
 
   private final Map<String, ApiInfo> currentPlugins = new HashMap<>();
 
+  private Phaser phaser;
+
   @Override
   public boolean onChange(Map<String, Object> properties) {
     refresh();
+    Phaser localPhaser = phaser; // volatile read
+    if (localPhaser != null) {
+      assert localPhaser.getRegisteredParties() == 1;
+      localPhaser.arrive(); // we should be the only ones registered, so this will advance phase each time
+    }
     return false;
   }
 
+  /**
+   * A phaser that will advance phases every time {@link #onChange(Map)} is called.
+   * Useful for allowing tests to know when a new configuration is finished getting set.
+   */
+
+  @VisibleForTesting
+  public void setPhaser(Phaser phaser) {
+    phaser.register();
+    this.phaser = phaser;
+  }
+
   public void registerListener(PluginRegistryListener listener) {
     listeners.add(listener);
   }

solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java

@@ -22,6 +22,8 @@ import java.io.InputStream;
 import java.nio.ByteBuffer;
 import java.util.Map;
 import java.util.concurrent.Callable;
+import java.util.concurrent.Phaser;
+import java.util.concurrent.TimeUnit;
 
 import com.google.common.collect.ImmutableMap;
 import org.apache.commons.io.IOUtils;
@@ -66,10 +68,12 @@ import static org.apache.solr.filestore.TestDistribPackageStore.readFile;
 import static org.apache.solr.filestore.TestDistribPackageStore.uploadKey;
 
 public class TestContainerPlugin extends SolrCloudTestCase {
+  private Phaser phaser;
 
   @Before
   public void setup() {
     System.setProperty("enable.packages", "true");
+    phaser = new Phaser();
   }
 
   @After
@@ -83,6 +87,11 @@ public class TestContainerPlugin extends SolrCloudTestCase {
             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();
@@ -100,6 +109,8 @@ public class TestContainerPlugin extends SolrCloudTestCase {
       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)
@@ -124,6 +135,8 @@ public class TestContainerPlugin extends SolrCloudTestCase {
               .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));
@@ -138,6 +151,7 @@ public class TestContainerPlugin extends SolrCloudTestCase {
       plugin.pathPrefix = "my-random-prefix";
 
       req.process(cluster.getSolrClient());
+      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
 
       //let's test the plugin
       TestDistribPackageStore.assertResponseValues(10,
@@ -161,6 +175,8 @@ public class TestContainerPlugin extends SolrCloudTestCase {
               .build()
               .process(cluster.getSolrClient());
 
+      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
       expectFail(() -> new V2Request.Builder("/my-random-prefix/their/plugin")
               .forceV2(true)
               .GET()
@@ -176,6 +192,8 @@ public class TestContainerPlugin extends SolrCloudTestCase {
       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")
@@ -208,6 +226,9 @@ public class TestContainerPlugin extends SolrCloudTestCase {
               .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)
@@ -222,6 +243,7 @@ public class TestContainerPlugin extends SolrCloudTestCase {
               .withPayload(singletonMap("update", p))
               .build()
               .process(cluster.getSolrClient());
+      version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
 
       TestDistribPackageStore.assertResponseValues(10,
               () -> new V2Request.Builder("hello/plugin")
@@ -254,6 +276,7 @@ public class TestContainerPlugin extends SolrCloudTestCase {
     }
     fail("should have failed with an exception");
   }
+
   @Test
   public void testApiFromPackage() throws Exception {
     MiniSolrCloudCluster cluster =
@@ -262,6 +285,11 @@ public class TestContainerPlugin extends SolrCloudTestCase {
                     .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 {
@@ -300,6 +328,8 @@ public class TestContainerPlugin extends SolrCloudTestCase {
               .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").
@@ -331,6 +361,7 @@ public class TestContainerPlugin extends SolrCloudTestCase {
               .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,
@@ -350,6 +381,7 @@ public class TestContainerPlugin extends SolrCloudTestCase {
       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 {
@@ -360,14 +392,15 @@ public class TestContainerPlugin extends SolrCloudTestCase {
   public static class CC1 extends CC {
 
   }
+
   public static class CC2 extends CC1 {
 
   }
+
   public static class CC implements ConfigurablePlugin<CConfig> {
     private CConfig cfg;
 
 
-
     @Override
     public void configure(CConfig cfg) {
       this.cfg = cfg;