You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ds...@apache.org on 2023/09/27 01:32:17 UTC
[solr] branch main updated: SOLR-16959: Make CoresLocator class configurable (#1891)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 708ab9b9be3 SOLR-16959: Make CoresLocator class configurable (#1891)
708ab9b9be3 is described below
commit 708ab9b9be31d8166a863eaa9e5f4a62d5da18f3
Author: Vincent P <vi...@gmail.com>
AuthorDate: Wed Sep 27 03:32:10 2023 +0200
SOLR-16959: Make CoresLocator class configurable (#1891)
Co-authored-by: Vincent Primault <vp...@salesforce.com>
---
solr/CHANGES.txt | 2 +
.../java/org/apache/solr/core/CoreContainer.java | 16 ++++----
.../apache/solr/core/CorePropertiesLocator.java | 11 ++++++
.../java/org/apache/solr/core/CoresLocator.java | 38 ++++++++++++++++---
.../src/java/org/apache/solr/core/NodeConfig.java | 18 ++++++++-
.../java/org/apache/solr/core/SolrXmlConfig.java | 3 ++
.../org/apache/solr/core/TestCoreContainer.java | 44 ++++++++++++++++++++++
.../test/org/apache/solr/core/TestLazyCores.java | 2 +-
.../pages/configuring-solr-xml.adoc | 14 ++++++-
.../org/apache/solr/util/ReadOnlyCoresLocator.java | 6 +++
.../src/java/org/apache/solr/util/TestHarness.java | 2 +-
11 files changed, 138 insertions(+), 18 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index e8382b835ff..6c084388b4e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -142,6 +142,8 @@ Improvements
* SOLR-15440: The Learning To Rank FieldValueFeature now uses DocValues when docValues=true and stored=true are combined. (Christine Poerschke, Tom Gilke)
+* SOLR-16959: Make the internal CoresLocator implementation configurable in solr.xml (Vincent Primault via David Smiley)
+
Optimizations
---------------------
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index b35cbbef058..cdf90bdc7a4 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -26,7 +26,6 @@ import static org.apache.solr.common.params.CommonParams.INFO_HANDLER_PATH;
import static org.apache.solr.common.params.CommonParams.METRICS_PATH;
import static org.apache.solr.common.params.CommonParams.ZK_PATH;
import static org.apache.solr.common.params.CommonParams.ZK_STATUS_PATH;
-import static org.apache.solr.core.CorePropertiesLocator.PROPERTIES_FILENAME;
import static org.apache.solr.security.AuthenticationPlugin.AUTHENTICATION_PLUGIN_PROP;
import com.github.benmanes.caffeine.cache.Interner;
@@ -377,21 +376,24 @@ public class CoreContainer {
* @see #load()
*/
public CoreContainer(NodeConfig config) {
- this(config, new CorePropertiesLocator(config.getCoreRootDirectory()));
+ this(config, CoresLocator.instantiate(config));
}
public CoreContainer(NodeConfig config, boolean asyncSolrCoreLoad) {
- this(config, new CorePropertiesLocator(config.getCoreRootDirectory()), asyncSolrCoreLoad);
+ this(config, CoresLocator.instantiate(config), asyncSolrCoreLoad);
}
/**
- * Create a new CoreContainer using the given configuration and locator. The container's cores are
- * not loaded.
+ * Create a new CoreContainer using the given configuration and locator.
+ *
+ * <p>The container's cores are not loaded. This constructor should be used only in tests, as it
+ * overrides {@link CoresLocator}'s instantiation process.
*
* @param config a ConfigSolr representation of this container's configuration
* @param locator a CoresLocator
* @see #load()
*/
+ @VisibleForTesting
public CoreContainer(NodeConfig config, CoresLocator locator) {
this(config, locator, false);
}
@@ -1945,9 +1947,7 @@ public class CoreContainer {
return null;
}
- CorePropertiesLocator cpl = new CorePropertiesLocator(null);
- CoreDescriptor ret =
- cpl.buildCoreDescriptor(oldDesc.getInstanceDir().resolve(PROPERTIES_FILENAME), this);
+ CoreDescriptor ret = getCoresLocator().reload(oldDesc, this);
// Ok, this little jewel is all because we still create core descriptors on the fly from lists
// of properties in tests particularly. Theoretically, there should be _no_ way to create a
diff --git a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
index be3da5c3447..c060fc5d3cd 100644
--- a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
+++ b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
@@ -16,6 +16,7 @@
*/
package org.apache.solr.core;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
@@ -51,6 +52,11 @@ public class CorePropertiesLocator implements CoresLocator {
private final Path rootDirectory;
+ public CorePropertiesLocator(NodeConfig nodeConfig) {
+ this(nodeConfig.getCoreRootDirectory());
+ }
+
+ @VisibleForTesting
public CorePropertiesLocator(Path coreDiscoveryRoot) {
this.rootDirectory = coreDiscoveryRoot;
log.debug("Config-defined core root directory: {}", this.rootDirectory);
@@ -193,6 +199,11 @@ public class CorePropertiesLocator implements CoresLocator {
return cds;
}
+ @Override
+ public CoreDescriptor reload(CoreDescriptor cd, CoreContainer cc) {
+ return buildCoreDescriptor(cd.getInstanceDir().resolve(PROPERTIES_FILENAME), cc);
+ }
+
protected CoreDescriptor buildCoreDescriptor(Path propertiesFile, CoreContainer cc) {
if (Files.notExists(propertiesFile)) {
// This can happen in tests, see CoreContainer#reloadCoreDescriptor
diff --git a/solr/core/src/java/org/apache/solr/core/CoresLocator.java b/solr/core/src/java/org/apache/solr/core/CoresLocator.java
index 10eac005230..3321e057860 100644
--- a/solr/core/src/java/org/apache/solr/core/CoresLocator.java
+++ b/solr/core/src/java/org/apache/solr/core/CoresLocator.java
@@ -27,7 +27,7 @@ public interface CoresLocator {
* @param cc the CoreContainer
* @param coreDescriptors CoreDescriptors to persist
*/
- public void create(CoreContainer cc, CoreDescriptor... coreDescriptors);
+ void create(CoreContainer cc, CoreDescriptor... coreDescriptors);
/**
* Ensure that the core definitions from the passed in CoreDescriptors will persist across
@@ -36,7 +36,7 @@ public interface CoresLocator {
* @param cc the CoreContainer
* @param coreDescriptors CoreDescriptors to persist
*/
- public void persist(CoreContainer cc, CoreDescriptor... coreDescriptors);
+ void persist(CoreContainer cc, CoreDescriptor... coreDescriptors);
/**
* Ensure that the core definitions from the passed in CoreDescriptors are not available for
@@ -45,7 +45,7 @@ public interface CoresLocator {
* @param cc the CoreContainer
* @param coreDescriptors CoreDescriptors of the cores to remove
*/
- public void delete(CoreContainer cc, CoreDescriptor... coreDescriptors);
+ void delete(CoreContainer cc, CoreDescriptor... coreDescriptors);
/**
* Persist the new name of a renamed core
@@ -54,7 +54,7 @@ public interface CoresLocator {
* @param oldCD the CoreDescriptor of the core before renaming
* @param newCD the CoreDescriptor of the core after renaming
*/
- public void rename(CoreContainer cc, CoreDescriptor oldCD, CoreDescriptor newCD);
+ void rename(CoreContainer cc, CoreDescriptor oldCD, CoreDescriptor newCD);
/**
* Swap two core definitions
@@ -63,7 +63,7 @@ public interface CoresLocator {
* @param cd1 the core descriptor of the first core, after swapping
* @param cd2 the core descriptor of the second core, after swapping
*/
- public void swap(CoreContainer cc, CoreDescriptor cd1, CoreDescriptor cd2);
+ void swap(CoreContainer cc, CoreDescriptor cd1, CoreDescriptor cd2);
/**
* Load all the CoreDescriptors from persistence store
@@ -71,5 +71,31 @@ public interface CoresLocator {
* @param cc the CoreContainer
* @return a list of all CoreDescriptors found
*/
- public List<CoreDescriptor> discover(CoreContainer cc);
+ List<CoreDescriptor> discover(CoreContainer cc);
+
+ /**
+ * Reload a core descriptor.
+ *
+ * @param cd the old core descriptor
+ * @param cc the CoreContainer
+ * @return a new core descriptor
+ */
+ CoreDescriptor reload(CoreDescriptor cd, CoreContainer cc);
+
+ /**
+ * Returns a new instance of {@link CoresLocator}, depending on provided config.
+ *
+ * @param nodeConfig Solr configuration.
+ */
+ static CoresLocator instantiate(NodeConfig nodeConfig) {
+ final String coresLocatorClass = nodeConfig.getCoresLocatorClass();
+ return nodeConfig
+ .getSolrResourceLoader()
+ .newInstance(
+ coresLocatorClass,
+ CoresLocator.class,
+ null,
+ new Class<?>[] {NodeConfig.class},
+ new Object[] {nodeConfig});
+ }
}
diff --git a/solr/core/src/java/org/apache/solr/core/NodeConfig.java b/solr/core/src/java/org/apache/solr/core/NodeConfig.java
index 9177889f447..387fe3beafb 100644
--- a/solr/core/src/java/org/apache/solr/core/NodeConfig.java
+++ b/solr/core/src/java/org/apache/solr/core/NodeConfig.java
@@ -56,6 +56,7 @@ public class NodeConfig {
private final String nodeName;
private final Path coreRootDirectory;
+ private final String coresLocatorClass;
private final Path solrDataHome;
@@ -125,6 +126,7 @@ public class NodeConfig {
private NodeConfig(
String nodeName,
Path coreRootDirectory,
+ String coresLocatorClass,
Path solrDataHome,
Integer booleanQueryMaxClauseCount,
Path configSetBaseDirectory,
@@ -162,6 +164,7 @@ public class NodeConfig {
// all Path params here are absolute and normalized.
this.nodeName = nodeName;
this.coreRootDirectory = coreRootDirectory;
+ this.coresLocatorClass = coresLocatorClass;
this.solrDataHome = solrDataHome;
this.booleanQueryMaxClauseCount = booleanQueryMaxClauseCount;
this.configSetBaseDirectory = configSetBaseDirectory;
@@ -271,6 +274,10 @@ public class NodeConfig {
return coreRootDirectory;
}
+ public String getCoresLocatorClass() {
+ return this.coresLocatorClass;
+ }
+
/** Absolute. */
public Path getSolrDataHome() {
return solrDataHome;
@@ -592,6 +599,7 @@ public class NodeConfig {
// all Path fields here are absolute and normalized.
private SolrResourceLoader loader;
private Path coreRootDirectory;
+ private String coresLocatorClass = DEFAULT_CORESLOCATORCLASS;
private Path solrDataHome;
private Integer booleanQueryMaxClauseCount;
private Path configSetBaseDirectory;
@@ -632,6 +640,8 @@ public class NodeConfig {
// No:of core load threads in cloud mode is set to a default of 8
public static final int DEFAULT_CORE_LOAD_THREADS_IN_CLOUD = 8;
+ private static final String DEFAULT_CORESLOCATORCLASS =
+ "org.apache.solr.core.CorePropertiesLocator";
private static final String DEFAULT_ADMINHANDLERCLASS =
"org.apache.solr.handler.admin.CoreAdminHandler";
private static final String DEFAULT_INFOHANDLERCLASS =
@@ -671,6 +681,11 @@ public class NodeConfig {
return this;
}
+ public NodeConfigBuilder setCoresLocatorClass(String coresLocatorClass) {
+ this.coresLocatorClass = coresLocatorClass;
+ return this;
+ }
+
public NodeConfigBuilder setSolrDataHome(String solrDataHomeString) {
// keep it null unless explicitly set to non-empty value
if (solrDataHomeString != null && !solrDataHomeString.isEmpty()) {
@@ -755,8 +770,8 @@ public class NodeConfig {
this.replayUpdatesThreads = replayUpdatesThreads;
return this;
}
-
// Remove in Solr 10.0
+
@Deprecated
public NodeConfigBuilder setTransientCacheSize(int transientCacheSize) {
this.transientCacheSize = transientCacheSize;
@@ -886,6 +901,7 @@ public class NodeConfig {
return new NodeConfig(
nodeName,
coreRootDirectory,
+ coresLocatorClass,
solrDataHome,
booleanQueryMaxClauseCount,
configSetBaseDirectory,
diff --git a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
index df0645c8f69..cba8a277504 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
@@ -348,6 +348,9 @@ public class SolrXmlConfig {
case "configSetService":
builder.setConfigSetServiceClass(it.txt());
break;
+ case "coresLocator":
+ builder.setCoresLocatorClass(it.txt());
+ break;
case "coreRootDirectory":
builder.setCoreRootDirectory(it.txt());
break;
diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
index e6b194858c9..5e828bc9be7 100644
--- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
+++ b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
@@ -770,6 +770,45 @@ public class TestCoreContainer extends SolrTestCaseJ4 {
}
}
+ @Test
+ public void testDefaultCoresLocator() throws Exception {
+ String solrXml = "<?xml version=\"1.0\" encoding=\"UTF-8\" ?><solr></solr>";
+ CoreContainer cc = init(solrXml);
+ try {
+ assertTrue(cc.getCoresLocator() instanceof CorePropertiesLocator);
+ } finally {
+ cc.shutdown();
+ }
+ }
+
+ @Test
+ public void testCustomCoresLocator() throws Exception {
+ String solrXml =
+ "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
+ + "<solr>\n"
+ + "<str name=\"coresLocator\">org.apache.solr.core.TestCoreContainer$CustomCoresLocator</str>\n"
+ + "</solr>";
+ CoreContainer cc = init(solrXml);
+ try {
+ assertTrue(cc.getCoresLocator() instanceof CustomCoresLocator);
+ assertSame(cc.getNodeConfig(), ((CustomCoresLocator) cc.getCoresLocator()).getNodeConfig());
+ } finally {
+ cc.shutdown();
+ }
+ }
+
+ public static class CustomCoresLocator extends MockCoresLocator {
+ private final NodeConfig nodeConfig;
+
+ public CustomCoresLocator(NodeConfig nodeConfig) {
+ this.nodeConfig = nodeConfig;
+ }
+
+ public NodeConfig getNodeConfig() {
+ return nodeConfig;
+ }
+ }
+
private static class MockCoresLocator implements CoresLocator {
List<CoreDescriptor> cores = new ArrayList<>();
@@ -799,6 +838,11 @@ public class TestCoreContainer extends SolrTestCaseJ4 {
public List<CoreDescriptor> discover(CoreContainer cc) {
return cores;
}
+
+ @Override
+ public CoreDescriptor reload(CoreDescriptor cd, CoreContainer cc) {
+ return cd;
+ }
}
@Test
diff --git a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
index fbb2f5ecf57..30b038609cb 100644
--- a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
+++ b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
@@ -743,7 +743,7 @@ public class TestLazyCores extends SolrTestCaseJ4 {
NodeConfig config = SolrXmlConfig.fromString(solrHomeDirectory.toPath(), "<solr/>");
// OK this should succeed, but at the end we should have recorded a series of errors.
- return createCoreContainer(config, new CorePropertiesLocator(config.getCoreRootDirectory()));
+ return createCoreContainer(config, new CorePropertiesLocator(config));
}
// We want to see that the core "heals itself" if an un-corrupted file is written to the
diff --git a/solr/solr-ref-guide/modules/configuration-guide/pages/configuring-solr-xml.adoc b/solr/solr-ref-guide/modules/configuration-guide/pages/configuring-solr-xml.adoc
index 09dc697ed49..65751c24c06 100644
--- a/solr/solr-ref-guide/modules/configuration-guide/pages/configuring-solr-xml.adoc
+++ b/solr/solr-ref-guide/modules/configuration-guide/pages/configuring-solr-xml.adoc
@@ -95,7 +95,7 @@ The tables below list the child nodes of each XML element in `solr.xml`.
+
This attribute does not need to be set.
+
-If used, this attribute should be set to the FQN (Fully qualified name) of a class that inherits from `ConfigSetService`, and you must provide a constructor with one parameter which the type is `org.apache.solr.core.CoreContainer`.
+If used, this attribute should be set to the FQN (Fully qualified name) of a class that inherits from `ConfigSetService`, and you must provide a constructor with one parameter of type `org.apache.solr.core.CoreContainer`.
For example, `<str name="configSetService">com.myorg.CustomConfigSetService</str>`.
+
If this attribute isn't set, Solr uses the default `configSetService`, with zookeeper aware of `org.apache.solr.cloud.ZkConfigSetService`, without zookeeper aware of `org.apache.solr.core.FileSystemConfigSetService`.
@@ -189,6 +189,18 @@ The default value is equal to the number of processors.
+
The root of the core discovery tree, defaults to `$SOLR_HOME`.
+`coresLocator`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: `org.apache.solr.core.CorePropertiesLocator`
+|===
++
+This attribute does not need to be set.
++
+If used, this attribute should be set to the FQN (Fully qualified name) of a class that implements `CoresLocator`, and you must provide a constructor with one parameter of type `org.apache.solr.core.NodeConfig`.
+For example, `<str name="coresLocator">com.myorg.CustomCoresLocator</str>`.
+
`managementPath`::
+
[%autowidth,frame=none]
diff --git a/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java b/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java
index 29c9c41b0eb..fc04418048e 100644
--- a/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java
+++ b/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java
@@ -46,4 +46,10 @@ public abstract class ReadOnlyCoresLocator implements CoresLocator {
public void swap(CoreContainer cc, CoreDescriptor cd1, CoreDescriptor cd2) {
// no-op
}
+
+ @Override
+ public CoreDescriptor reload(CoreDescriptor cd, CoreContainer cc) {
+ // no-op
+ return cd;
+ }
}
diff --git a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
index 2f8dc0121a7..37ac8cf0e38 100644
--- a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
+++ b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
@@ -175,7 +175,7 @@ public class TestHarness extends BaseTestHarness {
}
public TestHarness(NodeConfig nodeConfig) {
- this(nodeConfig, new CorePropertiesLocator(nodeConfig.getCoreRootDirectory()));
+ this(nodeConfig, new CorePropertiesLocator(nodeConfig));
}
/**