SOLR-17096: Cluster Singleton plugin support in solr.xml

solr/CHANGES.txt

@@ -21,7 +21,9 @@ Improvements
 
 * SOLR-16536: Replace OpenTracing instrumentation with OpenTelemetry (Alex Deparvu, janhoy)
 
-* SOLR-17096: Support for Cluster Singleton plugins with immutable deployments (Paul McArthur)
+* SOLR-17077: When a shard rejoins leader election, leave previous election only once to save unneeded calls to Zookeeper. (Pierre Salagnac)
+
+* SOLR-17096: solr.xml now supports declaring clusterSingleton plugins (Paul McArthur, David Smiley)
 
 Optimizations
 ---------------------

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

@@ -23,26 +23,26 @@
 import org.apache.solr.client.solrj.request.beans.PluginMeta;
 import org.apache.solr.handler.admin.ContainerPluginsApi;
 
-/** A source for Container Plugin configurations */
+/** A source for Cluster Plugin configurations */
 public interface ClusterPluginsSource {
 
   /**
-   * Get the Container Plugins Read Api for this plugin source
+   * Get the Read Api for this plugin source
    *
    * @return A {@link ContainerPluginsApi} Read Api for this plugin source
    */
   ContainerPluginsApi.Read getReadApi();
 
   /**
-   * Get the Container Plugins Edit Api for this plugin source, if it supports edit operations
+   * Get the Edit Api for this plugin source, if it supports edit operations
    *
    * @return A {@link ContainerPluginsApi} Edit Api for this plugin source, or null if the plugin
    *     source does not support editing the plugin configs
    */
   ContainerPluginsApi.Edit getEditApi();
 
   /**
-   * Get a map of container plugin configurations from this source, where keys are plugin names and
+   * Get a map of cluster plugin configurations from this source, where keys are plugin names and
    * values are {@link PluginMeta} plugin metadata.
    *
    * @return An immutable map of plugin configurations

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

@@ -17,26 +17,25 @@
 package org.apache.solr.api;
 
 import org.apache.solr.core.CoreContainer;
-import org.apache.solr.core.NodeConfig;
 import org.apache.solr.core.SolrResourceLoader;
-import org.apache.solr.util.plugin.NamedListInitializedPlugin;
+import org.apache.solr.util.EnvUtils;
 
 /**
  * Loads the {@link ClusterPluginsSource} depending on the declared implementation. The default
- * implementation is {@link ZkClusterPluginsSource}, but can be overridden by a system property, or
- * by declaring the implementation class in solr.xml
+ * implementation is {@link ZkClusterPluginsSource}, but can be overridden by teh {@link
+ * ContainerPluginsRegistry#MUTABLE_CLUSTER_PLUGINS} property
  */
-public class ClusterPluginsSourceConfigurator implements NamedListInitializedPlugin {
+public class ClusterPluginsSourceConfigurator {
 
   /**
    * Resolves the name of the class that will be used to provide cluster plugins.
    *
    * @return The name of the class to use as the {@link ClusterPluginsSource}
    */
   public static String resolveClassName() {
-    return NodeConfig.isImmutableConfig()
-        ? NodeConfigClusterPluginsSource.class.getName()
-        : ZkClusterPluginsSource.class.getName();
+    return EnvUtils.getPropAsBool(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, true)
+        ? ZkClusterPluginsSource.class.getName()
+        : NodeConfigClusterPluginsSource.class.getName();
   }
 
   public static ClusterPluginsSource loadClusterPluginsSource(

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

@@ -73,6 +73,8 @@
 public class ContainerPluginsRegistry implements ClusterPropertiesListener, MapWriter, Closeable {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
+  public static final String MUTABLE_CLUSTER_PLUGINS = "solr.cluster.plugin.mutable";
+
   private static final ObjectMapper mapper =
       SolrJacksonAnnotationInspector.createObjectMapper()
           .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)

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

@@ -18,7 +18,6 @@
 package org.apache.solr.api;
 
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Function;
@@ -71,24 +70,21 @@ public void persistPlugins(Function<Map<String, Object>, Map<String, Object>> mo
 
   private static Map<String, Object> readPlugins(final NodeConfig cfg) {
     Map<String, Object> pluginInfos = new HashMap<>();
-    PluginInfo[] clusterSingletons = cfg.getClusterPlugins();
-    if (clusterSingletons != null) {
-      Arrays.stream(clusterSingletons)
-          .forEach(
-              p -> {
-                Map<String, Object> pluginMap = new HashMap<>();
-                final String pluginName = getPluginName(p);
-                pluginMap.put("name", pluginName);
-                pluginMap.put("class", p.className);
-
-                if (p.initArgs.size() > 0) {
-                  Map<String, Object> config = new HashMap<>();
-                  p.initArgs.toMap(config);
-                  pluginMap.put("config", config);
-                }
-
-                pluginInfos.put(pluginName, pluginMap);
-              });
+    PluginInfo[] clusterPlugins = cfg.getClusterPlugins();
+    if (clusterPlugins != null) {
+      for (PluginInfo p : clusterPlugins) {
+        Map<String, Object> pluginMap = new HashMap<>();
+        final String pluginName = getPluginName(p);
+        pluginMap.put("name", pluginName);
+        pluginMap.put("class", p.className);
+
+        if (p.initArgs.size() > 0) {
+          Map<String, Object> config = p.initArgs.toMap(new HashMap<>());
+          pluginMap.put("config", config);
+        }
+
+        pluginInfos.put(pluginName, pluginMap);
+      }
     }
     return pluginInfos;
   }

solr/core/src/java/org/apache/solr/core/NodeConfig.java

@@ -35,6 +35,7 @@
 import java.util.stream.Collectors;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.solr.api.ClusterPluginsSourceConfigurator;
+import org.apache.solr.api.ContainerPluginsRegistry;
 import org.apache.solr.api.NodeConfigClusterPluginsSource;
 import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.SolrException;
@@ -52,8 +53,6 @@
 public class NodeConfig {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  public static final String CONFIG_EDITING_DISABLED_ARG = "disable.configEdit";
-
   // all Path fields here are absolute and normalized.
 
   private final String nodeName;
@@ -223,18 +222,14 @@ private NodeConfig(
       throw new SolrException(
           ErrorCode.SERVER_ERROR,
           "Cluster plugins found in solr.xml but the property "
-              + CONFIG_EDITING_DISABLED_ARG
-              + " is set to false. Cluster plugins may only be declared in solr.xml with immutable configs.");
+              + ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS
+              + " is set to true. Cluster plugins may only be declared in solr.xml with immutable configs.");
     }
 
     setupSharedLib();
     initModules();
   }
 
-  public static boolean isImmutableConfig() {
-    return Boolean.getBoolean(CONFIG_EDITING_DISABLED_ARG);
-  }
-
   /**
    * Get the NodeConfig. This may also be used by custom filters to load relevant configuration.
    *

solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java

@@ -160,8 +160,6 @@ public static NodeConfig fromConfig(
     configBuilder.setSolrResourceLoader(loader);
     configBuilder.setUpdateShardHandlerConfig(updateConfig);
     configBuilder.setShardHandlerFactoryConfig(getPluginInfo(root.get("shardHandlerFactory")));
-    // configBuilder.setReplicaPlacementFactoryConfig(
-    //    getPluginInfo(root.get("replicaPlacementFactory")));
     configBuilder.setTracerConfig(getPluginInfo(root.get("tracerConfig")));
     configBuilder.setLogWatcherConfig(loadLogWatcherConfig(root.get("logging")));
     configBuilder.setSolrProperties(loadProperties(root, substituteProperties));

solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java

@@ -79,7 +79,6 @@
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.ConfigOverlay;
-import org.apache.solr.core.NodeConfig;
 import org.apache.solr.core.PluginInfo;
 import org.apache.solr.core.RequestParams;
 import org.apache.solr.core.SolrConfig;
@@ -106,6 +105,9 @@
 public class SolrConfigHandler extends RequestHandlerBase
     implements SolrCoreAware, PermissionNameProvider {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+  public static final String CONFIGSET_EDITING_DISABLED_ARG = "disable.configEdit";
+  public static final boolean configEditing_disabled =
+      Boolean.getBoolean(CONFIGSET_EDITING_DISABLED_ARG);
   private static final Map<String, SolrConfig.SolrPluginInfo> namedPlugins;
   private final Lock reloadLock = new ReentrantLock(true);
 
@@ -133,10 +135,10 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
     String httpMethod = (String) req.getContext().get("httpMethod");
     Command command = new Command(req, rsp, httpMethod);
     if ("POST".equals(httpMethod)) {
-      if (NodeConfig.isImmutableConfig() || isImmutableConfigSet) {
+      if (configEditing_disabled || isImmutableConfigSet) {
         final String reason =
-            NodeConfig.isImmutableConfig()
-                ? "due to " + NodeConfig.CONFIG_EDITING_DISABLED_ARG
+            configEditing_disabled
+                ? "due to " + CONFIGSET_EDITING_DISABLED_ARG
                 : "because ConfigSet is immutable";
         throw new SolrException(
             SolrException.ErrorCode.FORBIDDEN, " solrconfig editing is not enabled " + reason);
@@ -210,9 +212,10 @@ private void handleGET() {
             resp.add(
                 ZNODEVER,
                 Map.of(
-                    ConfigOverlay.NAME, req.getCore().getSolrConfig().getOverlay().getVersion(),
+                    ConfigOverlay.NAME,
+                    req.getCore().getSolrConfig().getOverlay().getVersion(),
                     RequestParams.NAME,
-                        req.getCore().getSolrConfig().getRequestParams().getZnodeVersion()));
+                    req.getCore().getSolrConfig().getRequestParams().getZnodeVersion()));
             boolean isStale = false;
             int expectedVersion = req.getParams().getInt(ConfigOverlay.NAME, -1);
             int actualVersion = req.getCore().getSolrConfig().getOverlay().getVersion();

solr/core/src/test/org/apache/solr/api/NodeConfigClusterPluginsSourceTest.java

@@ -31,7 +31,6 @@
 import org.apache.solr.common.annotation.JsonProperty;
 import org.apache.solr.common.util.ReflectMapWriter;
 import org.apache.solr.core.CoreContainer;
-import org.apache.solr.core.NodeConfig;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -43,7 +42,7 @@ public class NodeConfigClusterPluginsSourceTest extends SolrCloudTestCase {
 
   @BeforeClass
   public static void setupCluster() throws Exception {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     CFG_VAL = random().nextInt();
     configureCluster(1)
         .withSolrXml(

solr/core/src/test/org/apache/solr/cluster/placement/impl/NodeConfigPlacementPluginTest.java

@@ -19,12 +19,12 @@
 
 import static org.hamcrest.Matchers.instanceOf;
 
+import org.apache.solr.api.ContainerPluginsRegistry;
 import org.apache.solr.cloud.MiniSolrCloudCluster;
 import org.apache.solr.cloud.SolrCloudTestCase;
 import org.apache.solr.cluster.placement.plugins.AffinityPlacementConfig;
 import org.apache.solr.cluster.placement.plugins.AffinityPlacementFactory;
 import org.apache.solr.core.CoreContainer;
-import org.apache.solr.core.NodeConfig;
 import org.hamcrest.MatcherAssert;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -33,7 +33,7 @@ public class NodeConfigPlacementPluginTest extends SolrCloudTestCase {
 
   @BeforeClass
   public static void setupCluster() throws Exception {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     String pluginXml =
         "<replicaPlacementFactory class=\"org.apache.solr.cluster.placement.plugins.AffinityPlacementFactory\"><int name=\"minimalFreeDiskGB\">10</int><int name=\"prioritizedFreeDiskGB\">200</int></replicaPlacementFactory>";
 

solr/core/src/test/org/apache/solr/core/TestSolrXml.java

@@ -30,6 +30,7 @@
 import org.apache.commons.exec.OS;
 import org.apache.lucene.tests.util.TestUtil;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.api.ContainerPluginsRegistry;
 import org.apache.solr.cloud.ClusterSingleton;
 import org.apache.solr.cluster.placement.plugins.AffinityPlacementFactory;
 import org.apache.solr.common.SolrException;
@@ -58,7 +59,7 @@ public void testAllInfoPresent() throws IOException {
 
     System.setProperty(
         "solr.allowPaths", OS.isFamilyWindows() ? "C:\\tmp,C:\\home\\john" : "/tmp,/home/john");
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     NodeConfig cfg = SolrXmlConfig.fromSolrHome(solrHome, new Properties());
     CloudConfig ccfg = cfg.getCloudConfig();
     UpdateShardHandlerConfig ucfg = cfg.getUpdateShardHandlerConfig();
@@ -165,7 +166,7 @@ public void testAllInfoPresent() throws IOException {
   // Test  a few property substitutions that happen to be in solr-50-all.xml.
   public void testPropertySub() throws IOException {
 
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     System.setProperty("coreRootDirectory", "myCoreRoot" + File.separator);
     System.setProperty("hostPort", "8888");
     System.setProperty("shareSchema", "false");
@@ -500,7 +501,7 @@ public void testMultiBackupSectionError() {
   }
 
   public void testFailAtConfigParseTimeWhenClusterSingletonHasNoName() {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     String solrXml =
         "<solr><clusterPluginsSource class=\"org.apache.solr.api.NodeConfigClusterPluginsSource\"/><clusterSingleton class=\"k1\"/></solr>";
     RuntimeException thrown =
@@ -511,7 +512,7 @@ public void testFailAtConfigParseTimeWhenClusterSingletonHasNoName() {
   }
 
   public void testFailAtConfigParseTimeWhenClusterSingletonHasDuplicateName() {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     String solrXml =
         "<solr><clusterPluginsSource class=\"org.apache.solr.api.NodeConfigClusterPluginsSource\"/><clusterSingleton name=\"a\" class=\"k1\"/><clusterSingleton name=\"a\" class=\"k2\"/></solr>";
     SolrException thrown =
@@ -521,7 +522,7 @@ public void testFailAtConfigParseTimeWhenClusterSingletonHasDuplicateName() {
   }
 
   public void testFailAtConfigParseTimeWhenClusterSingletonHasNoClass() {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     String solrXml =
         "<solr><clusterPluginsSource class=\"org.apache.solr.api.NodeConfigClusterPluginsSource\"/><clusterSingleton name=\"a\"/></solr>";
     RuntimeException thrown =
@@ -537,20 +538,20 @@ public void testFailAtConfigParseTimeWhenClusterSingletonWithWrongPluginsSource(
     SolrException thrown =
         assertThrows(SolrException.class, () -> SolrXmlConfig.fromString(solrHome, solrXml));
     assertEquals(
-        "Cluster plugins found in solr.xml but the property disable.configEdit is set to false. Cluster plugins may only be declared in solr.xml with immutable configs.",
+        "Cluster plugins found in solr.xml but the property solr.cluster.plugin.mutable is set to true. Cluster plugins may only be declared in solr.xml with immutable configs.",
         thrown.getMessage());
   }
 
   public void testFailAtConfigParseTimeWhenClusterSingletonClassNotFound() {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     String solrXml = "<solr><clusterSingleton name=\"a\" class=\"class.not.found.Class\"/></solr>";
     SolrException thrown =
         assertThrows(SolrException.class, () -> SolrXmlConfig.fromString(solrHome, solrXml));
     assertEquals(" Error loading class 'class.not.found.Class'", thrown.getMessage());
   }
 
   public void testFailAtConfigParseTimeWhenClusterSingletonClassHierarchyIllegal() {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     String solrXml =
         "<solr><clusterSingleton name=\"a\" class=\"" + NotCS.class.getName() + "\"/></solr>";
     SolrException thrown =
@@ -565,7 +566,7 @@ public void testFailAtConfigParseTimeWhenClusterSingletonClassHierarchyIllegal()
    * acceptable
    */
   public void testReplicaPlacementFactoryNameCanBeSet() {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     String solrXml =
         "<solr><replicaPlacementFactory name=\".placement-plugin\" class=\""
             + AffinityPlacementFactory.class.getName()
@@ -574,7 +575,7 @@ public void testReplicaPlacementFactoryNameCanBeSet() {
   }
 
   public void testFailAtConfigParseTimeWhenReplicaPlacementFactoryNameIsInvalid() {
-    System.setProperty(NodeConfig.CONFIG_EDITING_DISABLED_ARG, "true");
+    System.setProperty(ContainerPluginsRegistry.MUTABLE_CLUSTER_PLUGINS, "false");
     String solrXml =
         "<solr><replicaPlacementFactory name=\".must-be-placement-plugin\" class=\""
             + AffinityPlacementFactory.class.getName()

solr/solr-ref-guide/modules/configuration-guide/pages/cluster-plugins.adoc

@@ -23,7 +23,7 @@ Cluster plugins are pluggable components that are defined and instantiated at th
 These components usually provide admin-level functionality and APIs for additional functionality at the Solr node level.
 
 === Plugin Configurations
-If the `disable.configEdit` property is set to false, then plugin configurations can be maintained using `/cluster/plugin` API.
+If the `solr.cluster.plugin.mutable` property is set to true (the default), then plugin configurations can be maintained using `/cluster/plugin` API.
 
 This API endpoint allows adding, removing and updating plugin configurations.
 

solr/solr-ref-guide/modules/configuration-guide/pages/configuring-solr-xml.adoc

@@ -658,7 +658,7 @@ If only `dividend` routing is desired, `hash` may be explicitly set to the empty
 === The <replicaPlacementFactory> Element
 
 A default xref:replica-placement-plugins.adoc[replica placement plugin] can be defined in `solr.xml`.
-To allow this, the `disable.configEdit` System Property must be set to true. This setting will disable the `/cluster/plugins` edit APIs, preventing modification of cluster plugins at runtime.
+To allow this, the `solr.cluster.plugin.mutable` System Property must be set to false. This setting will disable the `/cluster/plugins` edit APIs, preventing modification of cluster plugins at runtime.
 
 [source,xml]
 ----
@@ -673,7 +673,7 @@ Sub-elements are specific to the implementation.
 
 === The <clusterSingleton> Element
 One or more `clusterSingleton` elements may be declared in solr.xml.
-To allow this, the `disable.configEdit` System Property must be set to true. This setting will disable the `/cluster/plugins` edit APIs, preventing modification of cluster plugins at runtime.
+To allow this, the `solr.cluster.plugin.mutable` System Property must be set to false. This setting will disable the `/cluster/plugins` edit APIs, preventing modification of cluster plugins at runtime.
 
 Each `clusterSingleton` element specifies a cluster plugin that should be loaded when Solr stars, along with it's associated configuration.