SOLR-14975: Optimize CoreContainer.getAllCoreNames and getLoadedCoreNames.

Also optimize getCoreDescriptors.

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<String> 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)" : "");
+  }
 }

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();

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;
             }

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<SolrCore> getCores() {
+  public List<SolrCore> 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.
    * <p>
    * 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.
+   * <p>
+   * 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<String> getLoadedCoreNames() {
+  public List<String> 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.
+   * <p>
+   * 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<String> getAllCoreNames() {
+  public List<String> 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)

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.*;
-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.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<String, SolrCore> cores = new LinkedHashMap<>(); // For "permanent" cores
 
   // These descriptors, once loaded, will _not_ be unloaded, i.e. they are not "transient".
-  private final Map<String, CoreDescriptor> residentDesciptors = new LinkedHashMap<>();
+  private final Map<String, CoreDescriptor> 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<SolrCore> 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);
-          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");
-        }
       } 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<SolrCore> coreList = new ArrayList<>();
 
-    
+    // Release transient core cache.
-    TransientSolrCoreCache transientSolrCoreCache = getTransientCacheHandler();
+    synchronized (modifyLock) {
-    // Release observer
+      if (transientSolrCoreCacheFactory != null) {
-    if (transientSolrCoreCache != null) {
+        getTransientCacheHandler().close();
-      transientSolrCoreCache.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) {
+        if (transientSolrCoreCacheFactory != null) {
-          coreList.addAll(transientSolrCoreCache.prepareForShutdown());
+          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<SolrCore> getCores() {
 
     synchronized (modifyLock) {
-      List<SolrCore> lst = new ArrayList<>(cores.values());
+      return new ArrayList<>(cores.values());
-      return lst;
     }
   }
 
@@ -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<String> getLoadedCoreNames() {
-    Set<String> 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<String> getAllCoreNames() {
+  List<String> getLoadedCoreNames() {
-    Set<String> set;
     synchronized (modifyLock) {
-      set = new TreeSet<>(cores.keySet());
+      return distinctSetsUnion(cores.keySet(), getTransientCacheHandler().getLoadedCoreNames());
-      if (getTransientCacheHandler() != null) {
+    }
-        set.addAll(getTransientCacheHandler().getAllCoreNames());
+  }
-      }
+
-      set.addAll(residentDesciptors.keySet());
+  /**
+   * 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<String> 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 <T> List<T> distinctSetsUnion(Set<T> set1, Set<T> set2) {
+    assert areSetsDistinct(set1, set2);
+    List<T> 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 <T> boolean areSetsDistinct(Set<T> set1, Set<T> 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) {
-      if (ret == null && transientHandler != null) {
+        ret = getTransientCacheHandler().removeCore(name);
-        ret = transientHandler.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 cores.containsKey(name) || getTransientCacheHandler().containsCore(name);
-        return true;
-      }
-      if (getTransientCacheHandler() != null && getTransientCacheHandler().containsCore(name)) {
-        return true;
-      }
     }
-    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))
+      CoreDescriptor coreDescriptor = residentDescriptors.get(coreName);
-        return residentDesciptors.get(coreName);
+      if (coreDescriptor != null) {
+        return coreDescriptor;
+      }
       return getTransientCacheHandler().getTransientDescriptor(coreName);
     }
   }
 
   /**
-   * Get the CoreDescriptors for every SolrCore managed here
+   * Get the CoreDescriptors for every {@link SolrCore} managed here (permanent and transient, loaded and unloaded).
-   * @return a List of CoreDescriptors
+   *
+   * @return An unordered list copy. This list can be modified by the caller (e.g. sorted).
    */
   public List<CoreDescriptor> getCoreDescriptors() {
-    List<CoreDescriptor> cds = Lists.newArrayList();
     synchronized (modifyLock) {
-      for (String coreName : getAllCoreNames()) {
+      Collection<CoreDescriptor> transientCoreDescriptors = getTransientCacheHandler().getTransientDescriptors();
-        // TODO: This null check is a bit suspicious - it seems that
+      List<CoreDescriptor> coreDescriptors = new ArrayList<>(residentDescriptors.size() + transientCoreDescriptors.size());
-        // getAllCoreNames might return deleted cores as well?
+      coreDescriptors.addAll(residentDescriptors.values());
-        CoreDescriptor cd = getCoreDescriptor(coreName);
+      coreDescriptors.addAll(transientCoreDescriptors);
-        if (cd != null)
+      return coreDescriptors;
-          cds.add(cd);
-      }
     }
-    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 currentlyLoadingCores.contains(name);
-      return true;
-    }
-    return false;
   }
 
   public void queueCoreToClose(SolrCore coreToClose) {
@@ -522,14 +540,16 @@ class SolrCores {
     }
   }
 
+  /**
+   * @return the cache holding the transient cores; never null.
+   */
   public TransientSolrCoreCache getTransientCacheHandler() {
-
+    synchronized (modifyLock) {
-    if (transientCoreCache == null) {
+      if (transientSolrCoreCacheFactory == null) {
-      log.error("No transient handler has been defined. Check solr.xml to see if an attempt to provide a custom {}"
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, getClass().getName() + " not loaded; call load() before using it");
-          , "TransientSolrCoreCacheFactory was done incorrectly since the default should have been used otherwise.");
+      }
-      return null;
+      return transientSolrCoreCacheFactory.getTransientSolrCoreCache();
     }
-    return transientCoreCache.getTransientSolrCoreCache();
   }
 
 }

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<CoreDescriptor> getTransientDescriptors();
 
   // Remove the core descriptor from your list of transient descriptors.
   public abstract CoreDescriptor removeTransientDescriptor(String name);

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.*;
-import java.util.Collection;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 import org.apache.solr.common.util.NamedList;
 import org.slf4j.Logger;
@@ -124,12 +119,12 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
 
   @Override
   public Set<String> getAllCoreNames() {
-    return transientDescriptors.keySet();
+    return Collections.unmodifiableSet(transientDescriptors.keySet());
   }
   
   @Override
   public Set<String> 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<CoreDescriptor> getTransientDescriptors() {
+    return Collections.unmodifiableCollection(transientDescriptors.values());
+  }
+
   @Override
   public CoreDescriptor removeTransientDescriptor(String name) {
     return transientDescriptors.remove(name);

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
+      // Many things could cause this, bad solrconfig, mis-typed class name, whatever.
-      // keep the enclosing coreContainer from instantiating, so log an error and continue.
+      // Throw an exception to stop loading here; never return null.
-      log.error("Error instantiating TransientSolrCoreCacheFactory class [{}]: ", info.className, e);
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error instantiating "
-      return null;
+              + 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(), 

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<List<CoreDescriptor>> descriptorsSupplier = () -> {
+        Supplier<List<CoreDescriptor>> descriptorsSupplier = () ->
-          List<CoreDescriptor> descriptors = new ArrayList<>(cc.getLoadedCoreNames().size());
+                cc.getCores().stream().map(SolrCore::getCoreDescriptor).collect(Collectors.toList());
-          Collection<SolrCore> cores = cc.getCores();
-          for (SolrCore core : cores) {
-            descriptors.add(core.getCoreDescriptor());
-          }
-          return descriptors;
-        };
 
         ZkController zkController = new ZkController(cc, zookeeperHost, zkClientConnectTimeout, config, descriptorsSupplier);
 

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");

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<String> 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);

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();

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

@@ -668,6 +668,7 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     Collection<String> 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<CoreDescriptor> 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));
     }
 
   }

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