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/11/30 23:32:06 UTC
(solr) branch main updated: SOLR-17083: Make CoreSorter class configurable in solr.xml (#2088)
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 7eceece7677 SOLR-17083: Make CoreSorter class configurable in solr.xml (#2088)
7eceece7677 is described below
commit 7eceece76771bdcd71245bcdfd5c0ae99ec980ee
Author: Vincent P <vi...@gmail.com>
AuthorDate: Fri Dec 1 00:32:00 2023 +0100
SOLR-17083: Make CoreSorter class configurable in solr.xml (#2088)
Co-authored-by: Vincent Primault <vp...@salesforce.com>
---
solr/CHANGES.txt | 6 ++-
.../java/org/apache/solr/core/CoreContainer.java | 15 ++++++-
.../src/java/org/apache/solr/core/CoreSorter.java | 52 +++++++++++-----------
.../src/java/org/apache/solr/core/NodeConfig.java | 15 +++++++
.../java/org/apache/solr/core/SolrXmlConfig.java | 3 ++
solr/core/src/test-files/solr/solr-50-all.xml | 1 +
.../test/org/apache/solr/core/CoreSorterTest.java | 2 +-
.../org/apache/solr/core/TestCoreContainer.java | 18 ++++++++
.../src/test/org/apache/solr/core/TestSolrXml.java | 1 +
.../pages/configuring-solr-xml.adoc | 17 ++++++-
10 files changed, 99 insertions(+), 31 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index fa799b81fdb..612a77215f0 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -83,8 +83,6 @@ New Features
* SOLR-16974: Circuit Breakers can now be configured globally (janhoy, Christine Poerschke)
-* SOLR-17079: Allow to declare replica placement plugins in solr.xml (Vincent Primault)
-
* SOLR-16743: When using TLS, Solr can now auto-reload the keystore and truststore without the need to restart the process.
This is enabled by default when running with TLS and can be disabled or configured in solr.in.sh (Houston Putman, Tomás Fernández Löbbe)
@@ -111,6 +109,10 @@ Improvements
* SOLR-17035: Add trace id to jetty thread names to improve debuggability via stack traces (Alex Deparvu)
+* SOLR-17079: Allow to declare replica placement plugins in solr.xml (Vincent Primault)
+
+* SOLR-16959: Make the internal CoreSorter implementation configurable in solr.xml (Vincent Primault)
+
Optimizations
---------------------
* SOLR-17084: LBSolrClient (used by CloudSolrClient) now returns the count of core tracked as not live AKA zombies
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 c8749f4c9e0..551608ca733 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -246,6 +246,7 @@ public class CoreContainer {
protected final SolrNodeKeyPair nodeKeyPair;
protected final CoresLocator coresLocator;
+ private volatile CoreSorter coreSorter;
private volatile String hostName;
@@ -1018,10 +1019,18 @@ public class CoreContainer {
metricManager.registry(SolrMetricManager.getRegistryName(SolrInfoBean.Group.node)),
SolrMetricManager.mkName(
"coreLoadExecutor", SolrInfoBean.Category.CONTAINER.toString(), "threadPool"));
+
+ coreSorter =
+ loader.newInstance(
+ cfg.getCoreSorterClass(),
+ CoreSorter.class,
+ null,
+ new Class<?>[] {CoreContainer.class},
+ new Object[] {this});
final List<Future<SolrCore>> futures = new ArrayList<>();
try {
List<CoreDescriptor> cds = coresLocator.discover(this);
- cds = CoreSorter.sortCores(this, cds);
+ cds = coreSorter.sort(cds);
checkForDuplicateCoreNames(cds);
status |= CORE_DISCOVERY_COMPLETE;
@@ -1465,6 +1474,10 @@ public class CoreContainer {
return coresLocator;
}
+ public CoreSorter getCoreSorter() {
+ return coreSorter;
+ }
+
protected SolrCore registerCore(
CoreDescriptor cd, SolrCore core, boolean registerInZk, boolean skipRecovery) {
if (core == null) {
diff --git a/solr/core/src/java/org/apache/solr/core/CoreSorter.java b/solr/core/src/java/org/apache/solr/core/CoreSorter.java
index 399526dfa65..b62167c5ec7 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreSorter.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreSorter.java
@@ -38,7 +38,7 @@ import org.apache.solr.common.cloud.Slice;
* replicas in the current node. This helps in avoiding leaderVote timeouts happening in other nodes
* of the cluster
*/
-public final class CoreSorter implements Comparator<CoreDescriptor> {
+public class CoreSorter {
private static final CountsForEachShard zero = new CountsForEachShard(0, 0, 0);
@@ -79,21 +79,35 @@ public final class CoreSorter implements Comparator<CoreDescriptor> {
return 0;
};
- /** Primary entry-point to sort the cores. */
- public static List<CoreDescriptor> sortCores(
- CoreContainer coreContainer, List<CoreDescriptor> descriptors) {
+ private final CoreContainer cc;
+
+ public CoreSorter(CoreContainer cc) {
+ this.cc = cc;
+ }
+
+ public List<CoreDescriptor> sort(List<CoreDescriptor> cds) {
// sort the cores if it is in SolrCloud. In standalone mode the order does not matter
- if (coreContainer.isZooKeeperAware()) {
- return descriptors.stream()
- .sorted(new CoreSorter().init(coreContainer, descriptors))
+ if (cc.isZooKeeperAware()) {
+ Map<String, CountsForEachShard> shardsVsReplicaCounts = computeShardsVsReplicaCounts(cds);
+ return cds.stream()
+ .sorted(
+ (cd1, cd2) -> {
+ String s1 = getShardName(cd1.getCloudDescriptor());
+ String s2 = getShardName(cd2.getCloudDescriptor());
+ if (s1 == null || s2 == null) return cd1.getName().compareTo(cd2.getName());
+ CountsForEachShard c1 = shardsVsReplicaCounts.get(s1);
+ CountsForEachShard c2 = shardsVsReplicaCounts.get(s2);
+ int result = countsComparator.compare(c1, c2);
+ return result == 0 ? s1.compareTo(s2) : result;
+ })
.collect(toList()); // new list
}
- return descriptors;
+ return cds;
}
- private final Map<String, CountsForEachShard> shardsVsReplicaCounts = new HashMap<>();
-
- CoreSorter init(CoreContainer cc, Collection<CoreDescriptor> coreDescriptors) {
+ private Map<String, CountsForEachShard> computeShardsVsReplicaCounts(
+ Collection<CoreDescriptor> coreDescriptors) {
+ Map<String, CountsForEachShard> shardsVsReplicaCounts = new HashMap<>();
String myNodeName = cc.getNodeConfig().getNodeName();
ClusterState state = cc.getZkController().getClusterState();
for (CoreDescriptor coreDescriptor : coreDescriptors) {
@@ -116,19 +130,7 @@ public final class CoreSorter implements Comparator<CoreDescriptor> {
}
shardsVsReplicaCounts.put(sliceName, c);
}
-
- return this;
- }
-
- @Override
- public int compare(CoreDescriptor cd1, CoreDescriptor cd2) {
- String s1 = getShardName(cd1.getCloudDescriptor());
- String s2 = getShardName(cd2.getCloudDescriptor());
- if (s1 == null || s2 == null) return cd1.getName().compareTo(cd2.getName());
- CountsForEachShard c1 = shardsVsReplicaCounts.get(s1);
- CountsForEachShard c2 = shardsVsReplicaCounts.get(s2);
- int result = countsComparator.compare(c1, c2);
- return result == 0 ? s1.compareTo(s2) : result;
+ return shardsVsReplicaCounts;
}
static class CountsForEachShard {
@@ -168,7 +170,7 @@ public final class CoreSorter implements Comparator<CoreDescriptor> {
}
}
- static String getShardName(CloudDescriptor cd) {
+ private static String getShardName(CloudDescriptor cd) {
return cd == null ? null : cd.getCollectionName() + "_" + cd.getShardId();
}
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 3745a922e4d..d970bab5bbd 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 Path coreRootDirectory;
private final String coresLocatorClass;
+ private final String coreSorterClass;
private final Path solrDataHome;
@@ -122,6 +123,7 @@ public class NodeConfig {
String nodeName,
Path coreRootDirectory,
String coresLocatorClass,
+ String coreSorterClass,
Path solrDataHome,
Integer booleanQueryMaxClauseCount,
Path configSetBaseDirectory,
@@ -160,6 +162,7 @@ public class NodeConfig {
this.nodeName = nodeName;
this.coreRootDirectory = coreRootDirectory;
this.coresLocatorClass = coresLocatorClass;
+ this.coreSorterClass = coreSorterClass;
this.solrDataHome = solrDataHome;
this.booleanQueryMaxClauseCount = booleanQueryMaxClauseCount;
this.configSetBaseDirectory = configSetBaseDirectory;
@@ -265,6 +268,10 @@ public class NodeConfig {
return this.coresLocatorClass;
}
+ public String getCoreSorterClass() {
+ return coreSorterClass;
+ }
+
/** Absolute. */
public Path getSolrDataHome() {
return solrDataHome;
@@ -555,6 +562,7 @@ public class NodeConfig {
private SolrResourceLoader loader;
private Path coreRootDirectory;
private String coresLocatorClass = DEFAULT_CORESLOCATORCLASS;
+ private String coreSorterClass = DEFAULT_CORESORTERCLASS;
private Path solrDataHome;
private Integer booleanQueryMaxClauseCount;
private Path configSetBaseDirectory;
@@ -597,6 +605,7 @@ public class NodeConfig {
private static final String DEFAULT_CORESLOCATORCLASS =
"org.apache.solr.core.CorePropertiesLocator";
+ private static final String DEFAULT_CORESORTERCLASS = "org.apache.solr.core.CoreSorter";
private static final String DEFAULT_ADMINHANDLERCLASS =
"org.apache.solr.handler.admin.CoreAdminHandler";
private static final String DEFAULT_INFOHANDLERCLASS =
@@ -641,6 +650,11 @@ public class NodeConfig {
return this;
}
+ public NodeConfigBuilder setCoreSorterClass(String coreSorterClass) {
+ this.coreSorterClass = coreSorterClass;
+ return this;
+ }
+
public NodeConfigBuilder setSolrDataHome(String solrDataHomeString) {
// keep it null unless explicitly set to non-empty value
if (solrDataHomeString != null && !solrDataHomeString.isEmpty()) {
@@ -859,6 +873,7 @@ public class NodeConfig {
nodeName,
coreRootDirectory,
coresLocatorClass,
+ coreSorterClass,
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 f9d5fecd7ec..aeae09b2212 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
@@ -336,6 +336,9 @@ public class SolrXmlConfig {
case "coresLocator":
builder.setCoresLocatorClass(it.txt());
break;
+ case "coreSorter":
+ builder.setCoreSorterClass(it.txt());
+ break;
case "coreRootDirectory":
builder.setCoreRootDirectory(it.txt());
break;
diff --git a/solr/core/src/test-files/solr/solr-50-all.xml b/solr/core/src/test-files/solr/solr-50-all.xml
index c0677d06977..0c212d72bd7 100644
--- a/solr/core/src/test-files/solr/solr-50-all.xml
+++ b/solr/core/src/test-files/solr/solr-50-all.xml
@@ -27,6 +27,7 @@
<str name="allowPaths">${solr.allowPaths:}</str>
<str name="shareSchema">${shareSchema:true}</str>
<str name="coresLocator">testCoresLocator</str>
+ <str name="coreSorter">testCoreSorter</str>
<int name="transientCacheSize">66</int>
<int name="replayUpdatesThreads">100</int>
<int name="maxBooleanClauses">42</int>
diff --git a/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java b/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
index 5e7458a6196..bbb55825445 100644
--- a/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
+++ b/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
@@ -186,7 +186,7 @@ public class CoreSorterTest extends SolrTestCaseJ4 {
for (int i = 0; i < 10; i++) {
Collections.shuffle(myDescs, random());
- List<CoreDescriptor> resultDescs = CoreSorter.sortCores(mockCC, myDescs);
+ List<CoreDescriptor> resultDescs = new CoreSorter(mockCC).sort(myDescs);
// map descriptors back to counts, removing consecutive duplicates
List<CountsForEachShard> resultCounts = new ArrayList<>();
CountsForEachShard lastCounts = null;
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 5e828bc9be7..e0d20b391c6 100644
--- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
+++ b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
@@ -845,6 +845,24 @@ public class TestCoreContainer extends SolrTestCaseJ4 {
}
}
+ @Test
+ public void testCustomCoreSorter() throws Exception {
+ String solrXml =
+ "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
+ + "<solr>\n"
+ + "<str name=\"coreSorter\">org.apache.solr.core.TestCoreContainer$CustomCoreSorter</str>\n"
+ + "</solr>";
+ CoreContainer cc = init(solrXml);
+ assertTrue(cc.getCoreSorter() instanceof CustomCoreSorter);
+ cc.shutdown();
+ }
+
+ public static class CustomCoreSorter extends CoreSorter {
+ public CustomCoreSorter(CoreContainer cc) {
+ super(cc);
+ }
+ }
+
@Test
public void testCoreInitFailuresFromEmptyContainer() throws Exception {
// reused state
diff --git a/solr/core/src/test/org/apache/solr/core/TestSolrXml.java b/solr/core/src/test/org/apache/solr/core/TestSolrXml.java
index 94a66a4c5c6..9140d580d55 100644
--- a/solr/core/src/test/org/apache/solr/core/TestSolrXml.java
+++ b/solr/core/src/test/org/apache/solr/core/TestSolrXml.java
@@ -79,6 +79,7 @@ public class TestSolrXml extends SolrTestCaseJ4 {
assertEquals(
"config set handler class", "testConfigSetsHandler", cfg.getConfigSetsHandlerClass());
assertEquals("cores locator class", "testCoresLocator", cfg.getCoresLocatorClass());
+ assertEquals("core sorter class", "testCoreSorter", cfg.getCoreSorterClass());
assertEquals("core load threads", 11, cfg.getCoreLoadThreadCount(false));
assertEquals("replay update threads", 100, cfg.getReplayUpdatesThreads());
MatcherAssert.assertThat(
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 83c29c670f9..b96413793de 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
@@ -196,8 +196,21 @@ The root of the core discovery tree, defaults to `$SOLR_HOME`.
+
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>`.
+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>` would configure a custom cores locator.
+
+`coreSorter`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: `org.apache.solr.core.CoreSorter`
+|===
++
+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 `CoreSorter`, and you must provide a constructor with one parameter of type `org.apache.solr.core.CoreContainer`.
+This service is used when Solr is starting to prioritize which cores should be loaded first.
+For example, `<str name="coresLocator">com.myorg.CustomCoresLocator</str>` would configure a custom core sorter.
`managementPath`::
+