This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit b301c710e4068b1a6f102acdd8f6196f06e76210 Author: Vincent P <[email protected]> AuthorDate: Thu Nov 30 18:32:00 2023 -0500 SOLR-17083: Make CoreSorter class configurable in solr.xml (#2088) Co-authored-by: Vincent Primault <[email protected]> --- solr/CHANGES.txt | 8 ++-- .../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 + solr/solr-ref-guide/build.gradle | 2 +- .../pages/configuring-solr-xml.adoc | 17 ++++++- 11 files changed, 100 insertions(+), 34 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 20ae71a470b..5d15d558a83 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -8,15 +8,11 @@ https://github.com/apache/solr/blob/main/solr/solr-ref-guide/modules/upgrade-not ================== 9.5.0 ================== New Features --------------------- -(No changes) - * SOLR-17006: Collection creation & adding replicas: User-defined properties are persisted to state.json and applied to new replicas, available for use as property substitution in configuration files. (Vincent Primault) * 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) - Improvements --------------------- * SOLR-17053: Distributed search with shards.tolerant: if all shards fail, fail the request (Aparna Suresh via David Smiley) @@ -42,6 +38,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 a4d3518fe5f..0f7a3748a4a 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -248,6 +248,7 @@ public class CoreContainer { protected final SolrNodeKeyPair nodeKeyPair; protected final CoresLocator coresLocator; + private volatile CoreSorter coreSorter; private volatile String hostName; @@ -1020,10 +1021,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; @@ -1469,6 +1478,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 2e9503022f2..c90137e214c 100644 --- a/solr/core/src/java/org/apache/solr/core/NodeConfig.java +++ b/solr/core/src/java/org/apache/solr/core/NodeConfig.java @@ -57,6 +57,7 @@ public class NodeConfig { private final Path coreRootDirectory; private final String coresLocatorClass; + private final String coreSorterClass; private final Path solrDataHome; @@ -127,6 +128,7 @@ public class NodeConfig { String nodeName, Path coreRootDirectory, String coresLocatorClass, + String coreSorterClass, Path solrDataHome, Integer booleanQueryMaxClauseCount, Path configSetBaseDirectory, @@ -166,6 +168,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; @@ -280,6 +283,10 @@ public class NodeConfig { return this.coresLocatorClass; } + public String getCoreSorterClass() { + return coreSorterClass; + } + /** Absolute. */ public Path getSolrDataHome() { return solrDataHome; @@ -606,6 +613,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; @@ -649,6 +657,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 = @@ -693,6 +702,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()) { @@ -916,6 +930,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 3a75f9683b1..678f9a17faa 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java +++ b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java @@ -353,6 +353,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 e1c34657c66..c65f2cc94f6 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 ea67bc5033d..db3d33b77ed 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/build.gradle b/solr/solr-ref-guide/build.gradle index fa73022ba5c..dcec2f01ce1 100644 --- a/solr/solr-ref-guide/build.gradle +++ b/solr/solr-ref-guide/build.gradle @@ -40,7 +40,7 @@ def escapeYamlSingleQuotesString(props) { // Attach building the ref guide to standard convention tasks. This // can be optionally turned off (see SOLR-15670). -var defaultRefGuideInclude = file("${rootDir}/.git").exists() +var defaultRefGuideInclude = file("${rootDir}/.git").isDirectory() if (propertyOrEnvOrDefault('refguide.include', 'SOLR_REF_GUIDE_INCLUDE', "${defaultRefGuideInclude}").toBoolean()) { check.dependsOn 'checkSiteLinks' assemble.dependsOn 'buildLocalSite' 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 1abb7dce387..168c54d46ca 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 @@ -198,8 +198,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`:: +
