From 67f9245ce30bb21d3976c05548856c81cf7ee8a1 Mon Sep 17 00:00:00 2001 From: Bruno Roustant Date: Fri, 6 Nov 2020 17:18:08 +0100 Subject: [PATCH] SOLR-14975: Optimize CoreContainer.getAllCoreNames and getLoadedCoreNames. Also optimize getCoreDescriptors. --- .../java/org/apache/solr/cloud/CloudUtil.java | 15 +- .../solr/cloud/ReplicateFromLeader.java | 3 +- .../cloud/ShardLeaderElectionContext.java | 2 +- .../org/apache/solr/core/CoreContainer.java | 41 ++- .../java/org/apache/solr/core/SolrCores.java | 238 ++++++++++-------- .../solr/core/TransientSolrCoreCache.java | 4 + .../core/TransientSolrCoreCacheDefault.java | 16 +- .../core/TransientSolrCoreCacheFactory.java | 16 +- .../org/apache/solr/core/ZkContainer.java | 13 +- .../handler/admin/HealthCheckHandler.java | 2 +- .../apache/solr/handler/admin/StatusOp.java | 5 +- .../apache/solr/core/TestCoreContainer.java | 3 + .../org/apache/solr/core/TestLazyCores.java | 10 +- .../client/solrj/request/TestCoreAdmin.java | 1 + 14 files changed, 216 insertions(+), 153 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/CloudUtil.java b/solr/core/src/java/org/apache/solr/cloud/CloudUtil.java index fc207311861..eefee37e578 100644 --- a/solr/core/src/java/org/apache/solr/cloud/CloudUtil.java +++ b/solr/core/src/java/org/apache/solr/cloud/CloudUtil.java @@ -29,6 +29,7 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import org.apache.commons.io.FileUtils; import org.apache.solr.client.solrj.cloud.SolrCloudManager; @@ -81,7 +82,7 @@ public class CloudUtil { if (thisCnn != null && thisCnn.equals(cnn) && !thisBaseUrl.equals(baseUrl)) { - if (cc.getLoadedCoreNames().contains(desc.getName())) { + if (cc.isLoaded(desc.getName())) { cc.unload(desc.getName()); } @@ -285,5 +286,15 @@ public class CloudUtil { }; } - + /** + * Builds a string with sorted {@link CoreContainer#getLoadedCoreNames()} while truncating to the first 20 cores. + */ + static String getLoadedCoreNamesAsString(CoreContainer coreContainer) { + List loadedCoreNames = coreContainer.getLoadedCoreNames(); + if (loadedCoreNames.size() <= 20) { + loadedCoreNames.sort(null); + } + return loadedCoreNames.stream().limit(20).collect(Collectors.toList()) + + (loadedCoreNames.size() > 20 ? "...(truncated from " + loadedCoreNames.size() + " cores)" : ""); + } } diff --git a/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java b/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java index 1fa86c0e5fa..7b18ffd773d 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java +++ b/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java @@ -69,7 +69,8 @@ public class ReplicateFromLeader { if (cc.isShutDown()) { return; } else { - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "SolrCore not found:" + coreName + " in " + cc.getLoadedCoreNames()); + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "SolrCore not found:" + coreName + " in " + + CloudUtil.getLoadedCoreNamesAsString(cc)); } } SolrConfig.UpdateHandlerInfo uinfo = core.getSolrConfig().getUpdateHandlerInfo(); diff --git a/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java b/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java index f6c96caf205..69ccf75c4ef 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java +++ b/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java @@ -288,7 +288,7 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase { if (core == null) { if (log.isDebugEnabled()) { - log.debug("SolrCore not found: {} in {}", coreName, cc.getLoadedCoreNames()); + log.debug("SolrCore not found: {} in {}", coreName, CloudUtil.getLoadedCoreNamesAsString(cc)); } return; } 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 e7751026bf1..7331cefcae0 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 java.security.spec.InvalidKeySpecException; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.HashMap; @@ -784,18 +783,18 @@ public class CoreContainer { // initialize gauges for reporting the number of cores and disk total/free - solrMetricsContext.gauge(() -> solrCores.getCores().size(), + solrMetricsContext.gauge(solrCores::getNumLoadedPermanentCores, true, "loaded", SolrInfoBean.Category.CONTAINER.toString(), "cores"); - solrMetricsContext.gauge(() -> solrCores.getLoadedCoreNames().size() - solrCores.getCores().size(), + solrMetricsContext.gauge(solrCores::getNumLoadedTransientCores, true, "lazy", SolrInfoBean.Category.CONTAINER.toString(), "cores"); - solrMetricsContext.gauge(() -> solrCores.getAllCoreNames().size() - solrCores.getLoadedCoreNames().size(), + solrMetricsContext.gauge(solrCores::getNumUnloadedCores, true, "unloaded", SolrInfoBean.Category.CONTAINER.toString(), "cores"); Path dataHome = cfg.getSolrDataHome() != null ? cfg.getSolrDataHome() : cfg.getCoreRootDirectory(); solrMetricsContext.gauge(() -> dataHome.toFile().getTotalSpace(), true, "totalSpace", SolrInfoBean.Category.CONTAINER.toString(), "fs"); solrMetricsContext.gauge(() -> dataHome.toFile().getUsableSpace(), true, "usableSpace", SolrInfoBean.Category.CONTAINER.toString(), "fs"); - solrMetricsContext.gauge(() -> dataHome.toString(), + solrMetricsContext.gauge(dataHome::toString, true, "path", SolrInfoBean.Category.CONTAINER.toString(), "fs"); solrMetricsContext.gauge(() -> cfg.getCoreRootDirectory().toFile().getTotalSpace(), true, "totalSpace", SolrInfoBean.Category.CONTAINER.toString(), "fs", "coreRoot"); @@ -1286,7 +1285,7 @@ public class CoreContainer { CoreDescriptor cd = new CoreDescriptor(coreName, instancePath, parameters, getContainerProperties(), getZkController()); // Since the core descriptor is removed when a core is unloaded, it should never be anywhere when a core is created. - if (getAllCoreNames().contains(coreName)) { + if (getCoreDescriptor(coreName) != null) { log.warn("Creating a core with existing name is not allowed: '{}'", coreName); // TODO: Shouldn't this be a BAD_REQUEST? throw new SolrException(ErrorCode.SERVER_ERROR, "Core with name '" + coreName + "' already exists."); @@ -1568,33 +1567,49 @@ public class CoreContainer { } /** - * @return a Collection of registered SolrCores + * Gets the permanent (non-transient) cores that are currently loaded. + * + * @return An unsorted list. This list is a new copy, it can be modified by the caller (e.g. it can be sorted). */ - public Collection getCores() { + public List getCores() { return solrCores.getCores(); } /** - * Gets the cores that are currently loaded, i.e. cores that have + * Gets the permanent and transient cores that are currently loaded, i.e. cores that have * 1: loadOnStartup=true and are either not-transient or, if transient, have been loaded and have not been aged out * 2: loadOnStartup=false and have been loaded but are either non-transient or have not been aged out. *

* Put another way, this will not return any names of cores that are lazily loaded but have not been called for yet * or are transient and either not loaded or have been swapped out. + *

+ * For efficiency, prefer to check {@link #isLoaded(String)} instead of {@link #getLoadedCoreNames()}.contains(coreName). + * + * @return An unsorted list. This list is a new copy, it can be modified by the caller (e.g. it can be sorted). */ - public Collection getLoadedCoreNames() { + public List getLoadedCoreNames() { return solrCores.getLoadedCoreNames(); } /** - * get a list of all the cores that are currently known, whether currently loaded or not + * Gets a collection of all the cores, permanent and transient, that are currently known, whether they are loaded or not. + *

+ * For efficiency, prefer to check {@link #getCoreDescriptor(String)} != null instead of {@link #getAllCoreNames()}.contains(coreName). * - * @return a list of all the available core names in either permanent or transient cores + * @return An unsorted list. This list is a new copy, it can be modified by the caller (e.g. it can be sorted). */ - public Collection getAllCoreNames() { + public List getAllCoreNames() { return solrCores.getAllCoreNames(); } + /** + * Gets the total number of cores, including permanent and transient cores, loaded and unloaded cores. + * Faster equivalent for {@link #getAllCoreNames()}.size(). + */ + public int getNumAllCores() { + return solrCores.getNumAllCores(); + } + /** * Returns an immutable Map of Exceptions that occurred when initializing * SolrCores (either at startup, or do to runtime requests to create cores) diff --git a/solr/core/src/java/org/apache/solr/core/SolrCores.java b/solr/core/src/java/org/apache/solr/core/SolrCores.java index 574bba0e723..4291b2ecbcc 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCores.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java @@ -16,37 +16,26 @@ */ package org.apache.solr.core; -import com.google.common.collect.Lists; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.ExecutorUtil; -import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.common.util.SolrNamedThreadFactory; +import org.apache.solr.logging.MDCLoggingContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.lang.invoke.MethodHandles; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.TreeSet; -import java.util.UUID; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; - class SolrCores { private static Object modifyLock = new Object(); // for locking around manipulating any of the core maps. private final Map cores = new LinkedHashMap<>(); // For "permanent" cores // These descriptors, once loaded, will _not_ be unloaded, i.e. they are not "transient". - private final Map residentDesciptors = new LinkedHashMap<>(); + private final Map residentDescriptors = new LinkedHashMap<>(); private final CoreContainer container; @@ -62,10 +51,8 @@ class SolrCores { // to essentially queue them up to be handled via pendingCoreOps. private static final List pendingCloses = new ArrayList<>(); - private TransientSolrCoreCacheFactory transientCoreCache; + private TransientSolrCoreCacheFactory transientSolrCoreCacheFactory; - private TransientSolrCoreCache transientSolrCoreCache = null; - SolrCores(CoreContainer container) { this.container = container; } @@ -73,13 +60,9 @@ class SolrCores { protected void addCoreDescriptor(CoreDescriptor p) { synchronized (modifyLock) { if (p.isTransient()) { - if (getTransientCacheHandler() != null) { - getTransientCacheHandler().addTransientDescriptor(p.getName(), p); - } else { - log.warn("We encountered a core marked as transient, but there is no transient handler defined. This core will be inaccessible"); - } + getTransientCacheHandler().addTransientDescriptor(p.getName(), p); } else { - residentDesciptors.put(p.getName(), p); + residentDescriptors.put(p.getName(), p); } } } @@ -87,29 +70,30 @@ class SolrCores { protected void removeCoreDescriptor(CoreDescriptor p) { synchronized (modifyLock) { if (p.isTransient()) { - if (getTransientCacheHandler() != null) { - getTransientCacheHandler().removeTransientDescriptor(p.getName()); - } + getTransientCacheHandler().removeTransientDescriptor(p.getName()); } else { - residentDesciptors.remove(p.getName()); + residentDescriptors.remove(p.getName()); } } } public void load(SolrResourceLoader loader) { - transientCoreCache = TransientSolrCoreCacheFactory.newInstance(loader, container); + synchronized (modifyLock) { + transientSolrCoreCacheFactory = TransientSolrCoreCacheFactory.newInstance(loader, container); + } } + // We are shutting down. You can't hold the lock on the various lists of cores while they shut down, so we need to // make a temporary copy of the names and shut them down outside the lock. protected void close() { waitForLoadingCoresToFinish(30*1000); Collection coreList = new ArrayList<>(); - - TransientSolrCoreCache transientSolrCoreCache = getTransientCacheHandler(); - // Release observer - if (transientSolrCoreCache != null) { - transientSolrCoreCache.close(); + // Release transient core cache. + synchronized (modifyLock) { + if (transientSolrCoreCacheFactory != null) { + getTransientCacheHandler().close(); + } } // It might be possible for one of the cores to move from one list to another while we're closing them. So @@ -121,8 +105,8 @@ class SolrCores { // make a copy of the cores then clear the map so the core isn't handed out to a request again coreList.addAll(cores.values()); cores.clear(); - if (transientSolrCoreCache != null) { - coreList.addAll(transientSolrCoreCache.prepareForShutdown()); + if (transientSolrCoreCacheFactory != null) { + coreList.addAll(getTransientCacheHandler().prepareForShutdown()); } coreList.addAll(pendingCloses); @@ -162,32 +146,28 @@ class SolrCores { addCoreDescriptor(cd); // cd must always be registered if we register a core if (cd.isTransient()) { - if (getTransientCacheHandler() != null) { - return getTransientCacheHandler().addCore(cd.getName(), core); - } + return getTransientCacheHandler().addCore(cd.getName(), core); } else { return cores.put(cd.getName(), core); } } - return null; } /** + * @return A list of "permanent" cores, i.e. cores that may not be swapped out and are currently loaded. * - * @return A list of "permanent" cores, i.e. cores that may not be swapped out and are currently loaded. - * * A core may be non-transient but still lazily loaded. If it is "permanent" and lazy-load _and_ * not yet loaded it will _not_ be returned by this call. - * - * Note: This is one of the places where SolrCloud is incompatible with Transient Cores. This call is used in + * + * This list is a new copy, it can be modified by the caller (e.g. it can be sorted). + * + * Note: This is one of the places where SolrCloud is incompatible with Transient Cores. This call is used in * cancelRecoveries, transient cores don't participate. */ - List getCores() { synchronized (modifyLock) { - List lst = new ArrayList<>(cores.values()); - return lst; + return new ArrayList<>(cores.values()); } } @@ -198,35 +178,86 @@ class SolrCores { * * Put another way, this will not return any names of cores that are lazily loaded but have not been called for yet * or are transient and either not loaded or have been swapped out. - * - * @return List of currently loaded cores. - */ - Set getLoadedCoreNames() { - Set set; - - synchronized (modifyLock) { - set = new TreeSet<>(cores.keySet()); - if (getTransientCacheHandler() != null) { - set.addAll(getTransientCacheHandler().getLoadedCoreNames()); - } - } - return set; - } - /** - * Gets a list of all cores, loaded and unloaded * - * @return all cores names, whether loaded or unloaded, transient or permanent. + * @return An unsorted list. This list is a new copy, it can be modified by the caller (e.g. it can be sorted). */ - public Collection getAllCoreNames() { - Set set; + List getLoadedCoreNames() { synchronized (modifyLock) { - set = new TreeSet<>(cores.keySet()); - if (getTransientCacheHandler() != null) { - set.addAll(getTransientCacheHandler().getAllCoreNames()); - } - set.addAll(residentDesciptors.keySet()); + return distinctSetsUnion(cores.keySet(), getTransientCacheHandler().getLoadedCoreNames()); + } + } + + /** + * Gets a collection of all cores names, loaded and unloaded. + * For efficiency, prefer to check {@link #getCoreDescriptor(String)} != null instead of {@link #getAllCoreNames()}.contains(String) + * + * @return An unsorted list. This list is a new copy, it can be modified by the caller (e.g. it can be sorted). + */ + public List getAllCoreNames() { + synchronized (modifyLock) { + return distinctSetsUnion(residentDescriptors.keySet(), getTransientCacheHandler().getAllCoreNames()); + } + } + + /** + * Makes the union of two distinct sets. + * + * @return An unsorted list. This list is a new copy, it can be modified by the caller (e.g. it can be sorted). + */ + private static List distinctSetsUnion(Set set1, Set set2) { + assert areSetsDistinct(set1, set2); + List union = new ArrayList<>(set1.size() + set2.size()); + union.addAll(set1); + union.addAll(set2); + return union; + } + + /** + * Indicates whether two sets are distinct (intersection is empty). + */ + private static boolean areSetsDistinct(Set set1, Set set2) { + return set1.stream().noneMatch(set2::contains); + } + + /** + * Gets the number of currently loaded permanent (non transient) cores. + * Faster equivalent for {@link #getCores()}.size(). + */ + int getNumLoadedPermanentCores() { + synchronized (modifyLock) { + return cores.size(); + } + } + + /** + * Gets the number of currently loaded transient cores. + */ + int getNumLoadedTransientCores() { + synchronized (modifyLock) { + return getTransientCacheHandler().getLoadedCoreNames().size(); + } + } + + /** + * Gets the number of unloaded cores, including permanent and transient cores. + */ + int getNumUnloadedCores() { + synchronized (modifyLock) { + assert areSetsDistinct(residentDescriptors.keySet(), getTransientCacheHandler().getAllCoreNames()); + return getTransientCacheHandler().getAllCoreNames().size() - getTransientCacheHandler().getLoadedCoreNames().size() + + residentDescriptors.size() - cores.size(); + } + } + + /** + * Gets the total number of cores, including permanent and transient cores, loaded and unloaded cores. + * Faster equivalent for {@link #getAllCoreNames()}.size(). + */ + public int getNumAllCores() { + synchronized (modifyLock) { + assert areSetsDistinct(residentDescriptors.keySet(), getTransientCacheHandler().getAllCoreNames()); + return residentDescriptors.size() + getTransientCacheHandler().getAllCoreNames().size(); } - return set; } SolrCore getCore(String name) { @@ -276,9 +307,8 @@ class SolrCores { SolrCore ret = cores.remove(name); // It could have been a newly-created core. It could have been a transient core. The newly-created cores // in particular should be checked. It could have been a dynamic core. - TransientSolrCoreCache transientHandler = getTransientCacheHandler(); - if (ret == null && transientHandler != null) { - ret = transientHandler.removeCore(name); + if (ret == null) { + ret = getTransientCacheHandler().removeCore(name); } return ret; } @@ -292,7 +322,7 @@ class SolrCores { synchronized (modifyLock) { SolrCore core = cores.get(name); - if (core == null && getTransientCacheHandler() != null) { + if (core == null) { core = getTransientCacheHandler().getCore(name); } if(core != null && coreId != null && coreId != core.uniqueId) return null; @@ -314,7 +344,7 @@ class SolrCores { if (cores.containsKey(name)) { return true; } - if (getTransientCacheHandler() != null && getTransientCacheHandler().containsCore(name)) { + if (getTransientCacheHandler().containsCore(name)) { // Check pending for (SolrCore core : pendingCloses) { if (core.getName().equals(name)) { @@ -330,22 +360,14 @@ class SolrCores { protected boolean isLoaded(String name) { synchronized (modifyLock) { - if (cores.containsKey(name)) { - return true; - } - if (getTransientCacheHandler() != null && getTransientCacheHandler().containsCore(name)) { - return true; - } + return cores.containsKey(name) || getTransientCacheHandler().containsCore(name); } - return false; - } protected CoreDescriptor getUnloadedCoreDescriptor(String cname) { synchronized (modifyLock) { - CoreDescriptor desc = residentDesciptors.get(cname); + CoreDescriptor desc = residentDescriptors.get(cname); if (desc == null) { - if (getTransientCacheHandler() == null) return null; desc = getTransientCacheHandler().getTransientDescriptor(cname); if (desc == null) { return null; @@ -432,28 +454,27 @@ class SolrCores { */ public CoreDescriptor getCoreDescriptor(String coreName) { synchronized (modifyLock) { - if (residentDesciptors.containsKey(coreName)) - return residentDesciptors.get(coreName); + CoreDescriptor coreDescriptor = residentDescriptors.get(coreName); + if (coreDescriptor != null) { + return coreDescriptor; + } return getTransientCacheHandler().getTransientDescriptor(coreName); } } /** - * Get the CoreDescriptors for every SolrCore managed here - * @return a List of CoreDescriptors + * Get the CoreDescriptors for every {@link SolrCore} managed here (permanent and transient, loaded and unloaded). + * + * @return An unordered list copy. This list can be modified by the caller (e.g. sorted). */ public List getCoreDescriptors() { - List cds = Lists.newArrayList(); synchronized (modifyLock) { - for (String coreName : getAllCoreNames()) { - // TODO: This null check is a bit suspicious - it seems that - // getAllCoreNames might return deleted cores as well? - CoreDescriptor cd = getCoreDescriptor(coreName); - if (cd != null) - cds.add(cd); - } + Collection transientCoreDescriptors = getTransientCacheHandler().getTransientDescriptors(); + List coreDescriptors = new ArrayList<>(residentDescriptors.size() + transientCoreDescriptors.size()); + coreDescriptors.addAll(residentDescriptors.values()); + coreDescriptors.addAll(transientCoreDescriptors); + return coreDescriptors; } - return cds; } // cores marked as loading will block on getCore @@ -509,10 +530,7 @@ class SolrCores { } public boolean isCoreLoading(String name) { - if (currentlyLoadingCores.contains(name)) { - return true; - } - return false; + return currentlyLoadingCores.contains(name); } public void queueCoreToClose(SolrCore coreToClose) { @@ -522,14 +540,16 @@ class SolrCores { } } + /** + * @return the cache holding the transient cores; never null. + */ public TransientSolrCoreCache getTransientCacheHandler() { - - if (transientCoreCache == null) { - log.error("No transient handler has been defined. Check solr.xml to see if an attempt to provide a custom {}" - , "TransientSolrCoreCacheFactory was done incorrectly since the default should have been used otherwise."); - return null; + synchronized (modifyLock) { + if (transientSolrCoreCacheFactory == null) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, getClass().getName() + " not loaded; call load() before using it"); + } + return transientSolrCoreCacheFactory.getTransientSolrCoreCache(); } - return transientCoreCache.getTransientSolrCoreCache(); } } diff --git a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java index 0947bd78380..77d5a65627e 100644 --- a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java +++ b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java @@ -97,6 +97,10 @@ public abstract class TransientSolrCoreCache { // method and return the current core descriptor. public abstract CoreDescriptor getTransientDescriptor(String name); + /** + * Gets the {@link CoreDescriptor} for all transient cores (loaded and unloaded). + */ + public abstract Collection getTransientDescriptors(); // Remove the core descriptor from your list of transient descriptors. public abstract CoreDescriptor removeTransientDescriptor(String name); diff --git a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java index 2ac94b10369..fcbfb0c8f67 100644 --- a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java +++ b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java @@ -18,12 +18,7 @@ package org.apache.solr.core; import java.lang.invoke.MethodHandles; -import java.util.ArrayList; -import java.util.Collection; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import org.apache.solr.common.util.NamedList; import org.slf4j.Logger; @@ -124,12 +119,12 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache { @Override public Set getAllCoreNames() { - return transientDescriptors.keySet(); + return Collections.unmodifiableSet(transientDescriptors.keySet()); } @Override public Set getLoadedCoreNames() { - return transientCores.keySet(); + return Collections.unmodifiableSet(transientCores.keySet()); } // Remove a core from the internal structures, presumably it @@ -166,6 +161,11 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache { return transientDescriptors.get(name); } + @Override + public Collection getTransientDescriptors() { + return Collections.unmodifiableCollection(transientDescriptors.values()); + } + @Override public CoreDescriptor removeTransientDescriptor(String name) { return transientDescriptors.remove(name); diff --git a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheFactory.java b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheFactory.java index 981058e0645..5a2631ff49d 100644 --- a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheFactory.java +++ b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheFactory.java @@ -20,6 +20,7 @@ import java.lang.invoke.MethodHandles; import java.util.Collections; import com.google.common.collect.ImmutableMap; +import org.apache.solr.common.SolrException; import org.apache.solr.util.plugin.PluginInfoInitialized; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,6 +33,9 @@ public abstract class TransientSolrCoreCacheFactory { private volatile CoreContainer coreContainer = null; + /** + * @return the cache holding the transient cores; never null. + */ public abstract TransientSolrCoreCache getTransientSolrCoreCache(); /** * Create a new TransientSolrCoreCacheFactory instance @@ -51,19 +55,19 @@ public abstract class TransientSolrCoreCacheFactory { // According to the docs, this returns a TransientSolrCoreCacheFactory with the default c'tor TransientSolrCoreCacheFactory tccf = loader.findClass(info.className, TransientSolrCoreCacheFactory.class).getConstructor().newInstance(); - // OK, now we call it's init method. + // OK, now we call its init method. if (PluginInfoInitialized.class.isAssignableFrom(tccf.getClass())) PluginInfoInitialized.class.cast(tccf).init(info); tccf.setCoreContainer(coreContainer); return tccf; } catch (Exception e) { - // Many things could cause this, bad solrconfig, mis-typed class name, whatever. However, this should not - // keep the enclosing coreContainer from instantiating, so log an error and continue. - log.error("Error instantiating TransientSolrCoreCacheFactory class [{}]: ", info.className, e); - return null; + // Many things could cause this, bad solrconfig, mis-typed class name, whatever. + // Throw an exception to stop loading here; never return null. + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error instantiating " + + TransientSolrCoreCacheFactory.class.getName() + " class [" + info.className + "]", e); } - } + public static final PluginInfo DEFAULT_TRANSIENT_SOLR_CACHE_INFO = new PluginInfo("transientSolrCoreCacheFactory", ImmutableMap.of("class", TransientSolrCoreCacheFactoryDefault.class.getName(), diff --git a/solr/core/src/java/org/apache/solr/core/ZkContainer.java b/solr/core/src/java/org/apache/solr/core/ZkContainer.java index 2ca62f8b37d..eb7c850c0da 100644 --- a/solr/core/src/java/org/apache/solr/core/ZkContainer.java +++ b/solr/core/src/java/org/apache/solr/core/ZkContainer.java @@ -22,13 +22,12 @@ import java.lang.invoke.MethodHandles; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeoutException; import java.util.function.Predicate; import java.util.function.Supplier; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.solr.cloud.SolrZkServer; @@ -125,14 +124,8 @@ public class ZkContainer { "A chroot was specified in ZkHost but the znode doesn't exist. " + zookeeperHost); } - Supplier> descriptorsSupplier = () -> { - List descriptors = new ArrayList<>(cc.getLoadedCoreNames().size()); - Collection cores = cc.getCores(); - for (SolrCore core : cores) { - descriptors.add(core.getCoreDescriptor()); - } - return descriptors; - }; + Supplier> descriptorsSupplier = () -> + cc.getCores().stream().map(SolrCore::getCoreDescriptor).collect(Collectors.toList()); ZkController zkController = new ZkController(cc, zookeeperHost, zkClientConnectTimeout, config, descriptorsSupplier); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/HealthCheckHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/HealthCheckHandler.java index 21a8d643754..1365a017275 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/HealthCheckHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/HealthCheckHandler.java @@ -126,7 +126,7 @@ public class HealthCheckHandler extends RequestHandlerBase { rsp.add(STATUS, FAILURE); rsp.add("num_cores_unhealthy", unhealthyCores); rsp.setException(new SolrException(SolrException.ErrorCode.SERVICE_UNAVAILABLE, unhealthyCores + " out of " - + cores.getAllCoreNames().size() + " replicas are currently initializing or recovering")); + + cores.getNumAllCores() + " replicas are currently initializing or recovering")); return; } rsp.add("message", "All cores are healthy"); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/StatusOp.java b/solr/core/src/java/org/apache/solr/handler/admin/StatusOp.java index f2bddbd9ec7..d1dadfcd160 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/StatusOp.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/StatusOp.java @@ -19,6 +19,7 @@ package org.apache.solr.handler.admin; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.solr.common.params.CoreAdminParams; @@ -42,7 +43,9 @@ class StatusOp implements CoreAdminHandler.CoreAdminOp { failures.put(failure.getKey(), failure.getValue().exception); } if (cname == null) { - for (String name : it.handler.coreContainer.getAllCoreNames()) { + List nameList = it.handler.coreContainer.getAllCoreNames(); + nameList.sort(null); + for (String name : nameList) { status.add(name, CoreAdminOperation.getCoreStatus(it.handler.coreContainer, name, isIndexInfoNeeded)); } it.rsp.add("initFailures", failures); 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 6c5093222d3..d05a9174846 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java +++ b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java @@ -670,6 +670,7 @@ public class TestCoreContainer extends SolrTestCaseJ4 { // check that we get null accessing a non-existent core assertNull(cc.getCore("does_not_exist")); + assertFalse(cc.isLoaded("does_not_exist")); // check that we get a 500 accessing the core with an init failure SolrException thrown = expectThrows(SolrException.class, () -> { SolrCore c = cc.getCore("col_bad"); @@ -691,7 +692,9 @@ public class TestCoreContainer extends SolrTestCaseJ4 { assertNotNull("core names is null", cores); assertEquals("wrong number of cores", 2, cores.size()); assertTrue("col_ok not found", cores.contains("col_ok")); + assertTrue(cc.isLoaded("col_ok")); assertTrue("col_bad not found", cores.contains("col_bad")); + assertTrue(cc.isLoaded("col_bad")); // check that we have the failures we expect failures = cc.getCoreInitFailures(); 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 a41b750e115..ff74d61fc56 100644 --- a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java +++ b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java @@ -668,6 +668,7 @@ public class TestLazyCores extends SolrTestCaseJ4 { Collection loadedNames = cc.getLoadedCoreNames(); for (String name : nameCheck) { assertFalse("core " + name + " was found in the list of cores", loadedNames.contains(name)); + assertFalse(cc.isLoaded(name)); } // There was a problem at one point exacerbated by the poor naming conventions. So parallel to loaded cores, there @@ -681,26 +682,33 @@ public class TestLazyCores extends SolrTestCaseJ4 { List descriptors = cc.getCoreDescriptors(); assertEquals("There should be as many coreDescriptors as coreNames", allNames.size(), descriptors.size()); + assertEquals(allNames.size(), cc.getNumAllCores()); for (CoreDescriptor desc : descriptors) { assertTrue("Name should have a corresponding descriptor", allNames.contains(desc.getName())); + assertNotNull(cc.getCoreDescriptor(desc.getName())); } // First check that all loaded cores are in allNames. for (String name : loadedNames) { assertTrue("Loaded core " + name + " should have been found in the list of all possible core names", allNames.contains(name)); + assertNotNull(cc.getCoreDescriptor(name)); + assertTrue(cc.isLoaded(name)); } - // failed cores should have had their descriptors removed. + // Unloaded cores should be in allNames. for (String name : nameCheck) { assertTrue("Not-currently-loaded core " + name + " should have been found in the list of all possible core names", allNames.contains(name)); + assertNotNull(cc.getCoreDescriptor(name)); } // Failed cores should not be in coreDescriptors. for (String name : namesBad) { assertFalse("Failed core " + name + " should have been found in the list of all possible core names", allNames.contains(name)); + assertNull(cc.getCoreDescriptor(name)); + assertFalse(cc.isLoaded(name)); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java index fb0073a6e78..1b4ecc326d5 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java @@ -193,6 +193,7 @@ public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase { names = cores.getAllCoreNames(); assertFalse(names.toString(), names.contains("coreRenamed")); assertTrue(names.toString(), names.contains("core1")); + assertEquals(names.size(), cores.getNumAllCores()); } @Test