You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by br...@apache.org on 2021/06/11 08:45:45 UTC

[solr] branch main updated: SOLR-15433: Replace transient core cache LRU by Caffeine cache.

This is an automated email from the ASF dual-hosted git repository.

broustant 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 26d3f95  SOLR-15433: Replace transient core cache LRU by Caffeine cache.
26d3f95 is described below

commit 26d3f951623810efbc563ea04470d384f84ba590
Author: Bruno Roustant <br...@gmail.com>
AuthorDate: Tue May 25 15:14:41 2021 +0200

    SOLR-15433: Replace transient core cache LRU by Caffeine cache.
---
 solr/CHANGES.txt                                   |   2 +-
 .../java/org/apache/solr/core/CoreContainer.java   |  10 +-
 .../apache/solr/core/TransientSolrCoreCache.java   |  59 ++++--
 .../solr/core/TransientSolrCoreCacheDefault.java   | 151 +++++++--------
 .../org/apache/solr/core/TestCoreDiscovery.java    |   7 +-
 .../test/org/apache/solr/core/TestLazyCores.java   | 208 ++++++++++++---------
 6 files changed, 242 insertions(+), 195 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index dd93909..0e9a27b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -342,7 +342,7 @@ Improvements
 
 Optimizations
 ---------------------
-(No changes)
+* SOLR-15433: Replace transient core cache LRU by Caffeine cache. (Bruno Roustant)
 
 Bug Fixes
 ---------------------
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 d6801e4..820a4b1 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -2269,13 +2269,13 @@ class CloserThread extends Thread {
           // any cores to close.
         }
       }
-      for (SolrCore removeMe = solrCores.getCoreToClose();
-           removeMe != null && !container.isShutDown();
-           removeMe = solrCores.getCoreToClose()) {
+
+      SolrCore core;
+      while (!container.isShutDown() && (core = solrCores.getCoreToClose()) != null) {
         try {
-          removeMe.close();
+          core.close();
         } finally {
-          solrCores.removeFromPendingOps(removeMe.getName());
+          solrCores.removeFromPendingOps(core.getName());
         }
       }
     }
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 77d5a65..a73fbce 100644
--- a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java
+++ b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java
@@ -62,39 +62,49 @@ import java.util.Set;
  */
 public abstract class TransientSolrCoreCache {
 
-  // Gets the core container that encloses this cache.
+  /** Gets the core container that encloses this cache. */
   public abstract CoreContainer getContainer();
 
-  // Add the newly-opened core to the list of open cores.
+  /** Adds the newly-opened core to the list of open cores. */
   public abstract SolrCore addCore(String name, SolrCore core);
 
-  // Return the names of all possible cores, whether they are currently loaded or not.
+  /** Returns the names of all possible cores, whether they are currently loaded or not. */
   public abstract Set<String> getAllCoreNames();
   
-  // Return the names of all currently loaded cores
+  /** Returns the names of all currently loaded cores. */
   public abstract Set<String> getLoadedCoreNames();
 
-  // Remove a core from the internal structures, presumably it 
-  // being closed. If the core is re-opened, it will be readded by CoreContainer.
+  /**
+   * Removes a core from the internal structures, presumably it being closed. If the core
+   * is re-opened, it will be re-added by CoreContainer.
+   */
   public abstract SolrCore removeCore(String name);
 
-  // Get the core associated with the name. Return null if you don't want this core to be used.
+  /** Gets the core associated with the name. Returns null if there is none. */
   public abstract SolrCore getCore(String name);
 
-  // reutrn true if the cache contains the named core.
+  /** Returns whether the cache contains the named core. */
   public abstract boolean containsCore(String name);
-  
-  // This method will be called when the container is to be shut down. It should return all
-  // transient solr cores and clear any internal structures that hold them.
+
+  /**
+   * This method will be called when the container is to be shut down. It returns
+   * all transient solr cores and clear any internal structures that hold them.
+   */
   public abstract Collection<SolrCore> prepareForShutdown();
 
   // These methods allow the implementation to maintain control over the core descriptors.
-  
-  // This method will only be called during core discovery at startup.
+
+  /**
+   * Adds a new {@link CoreDescriptor}.
+   * This method will only be called during core discovery at startup.
+   */
   public abstract void addTransientDescriptor(String rawName, CoreDescriptor cd);
-  
-  // This method is used when opening cores and the like. If you want to change a core's descriptor, override this
-  // method and return the current core descriptor.
+
+  /**
+   * Gets the {@link CoreDescriptor} for a transient core (loaded or unloaded).
+   * This method is used when opening cores and the like. If you want to change a core's descriptor,
+   * override this method and return the current core descriptor.
+   */
   public abstract CoreDescriptor getTransientDescriptor(String name);
 
   /**
@@ -102,19 +112,28 @@ public abstract class TransientSolrCoreCache {
    */
   public abstract Collection<CoreDescriptor> getTransientDescriptors();
 
-  // Remove the core descriptor from your list of transient descriptors.
+  /**
+   * Removes a {@link CoreDescriptor} from the list of transient cores descriptors.
+   */
   public abstract CoreDescriptor removeTransientDescriptor(String name);
 
   /**
-   * Must be called in order to free resources!
+   * Called in order to free resources.
    */
   public  void close(){
     // Nothing to do currently
   };
 
-
-  // These two methods allow custom implementations to communicate arbitrary information as necessary.
+  /**
+   * Gets a custom status for the given core name.
+   * Allows custom implementations to communicate arbitrary information as necessary.
+   */
   public abstract int getStatus(String coreName);
+
+  /**
+   * Sets a custom status for the given core name.
+   * Allows custom implementations to communicate arbitrary information as necessary.
+   */
   public abstract void setStatus(String coreName, int status);
 }
 
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 5e3bfd3..878df71 100644
--- a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java
+++ b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java
@@ -18,100 +18,114 @@
 package org.apache.solr.core;
 
 import java.lang.invoke.MethodHandles;
-import java.util.*;
-
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
 import org.apache.solr.common.util.NamedList;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/**
+ * Cache of the most frequently accessed transient cores. Keeps track of all the registered
+ * transient cores descriptors, including the cores in the cache as well as all the others.
+ */
 public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  private int cacheSize = NodeConfig.NodeConfigBuilder.DEFAULT_TRANSIENT_CACHE_SIZE;
+  protected final CoreContainer coreContainer;
 
-  protected CoreContainer coreContainer;
-
-  protected final Map<String, CoreDescriptor> transientDescriptors = new LinkedHashMap<>();
+  /**
+   * "Lazily loaded" cores cache with limited size. When the max size is reached, the least
+   * accessed core is evicted to make room for a new core.
+   */
+  protected final Cache<String, SolrCore> transientCores;
 
-  //WARNING! The _only_ place you put anything into the list of transient cores is with the putTransientCore method!
-  protected Map<String, SolrCore> transientCores = new LinkedHashMap<>(); // For "lazily loaded" cores
+  /**
+   * Unlimited map of all the descriptors for all the registered transient cores, including the
+   * cores in the {@link #transientCores} as well as all the others.
+   */
+  protected final Map<String, CoreDescriptor> transientDescriptors;
 
   /**
-   * @param container The enclosing CoreContainer. It allows us to access everything we need.
+   * @param coreContainer The enclosing {@link CoreContainer}.
    */
-  public TransientSolrCoreCacheDefault(final CoreContainer container) {
-    this.coreContainer = container;
+  public TransientSolrCoreCacheDefault(CoreContainer coreContainer) {
+    this.coreContainer = coreContainer;
+    int cacheMaxSize = getConfiguredCacheMaxSize(coreContainer);
 
-    NodeConfig cfg = container.getNodeConfig();
-    if (cfg.getTransientCachePluginInfo() == null) {
-      // Still handle just having transientCacheSize defined in the body of solr.xml  not in a transient handler clause.
-      // deprecate this for 7.0?
-      this.cacheSize = cfg.getTransientCacheSize();
-    } else {
-      NamedList<?> args = cfg.getTransientCachePluginInfo().initArgs;
-      Object obj = args.get("transientCacheSize");
-      if (obj != null) {
-        this.cacheSize = (int) obj;
-      }
+    // Now don't allow ridiculous allocations here, if the size is > 1,000, we'll just deal with
+    // adding cores as they're opened. This blows up with the marker value of -1.
+    int initialCapacity = Math.min(cacheMaxSize, 1024);
+    log.info("Allocating transient core cache for max {} cores with initial capacity of {}", cacheMaxSize, initialCapacity);
+    Caffeine<String, SolrCore> transientCoresCacheBuilder =
+        Caffeine.newBuilder()
+            .initialCapacity(initialCapacity)
+            // Use the current thread to queue evicted cores for closing. This ensures the
+            // cache max size is respected (with a different thread the max size would be
+            // respected asynchronously only eventually).
+            .executor(Runnable::run)
+            .removalListener(
+                (coreName, core, cause) -> {
+                  if (core != null && cause.wasEvicted()) {
+                    if (log.isInfoEnabled()) {
+                      log.info("Closing transient core [{}] evicted from the cache", core.getName());
+                    }
+                    coreContainer.queueCoreToClose(core);
+                  }
+                });
+    if (cacheMaxSize != Integer.MAX_VALUE) {
+      transientCoresCacheBuilder.maximumSize(cacheMaxSize);
     }
-    doInit();
+    transientCores = transientCoresCacheBuilder.build();
+
+    transientDescriptors = new LinkedHashMap<>(initialCapacity);
   }
-  // This just moves the 
-  private void doInit() {
-    NodeConfig cfg = coreContainer.getNodeConfig();
+
+  private int getConfiguredCacheMaxSize(CoreContainer container) {
+    int configuredCacheMaxSize = NodeConfig.NodeConfigBuilder.DEFAULT_TRANSIENT_CACHE_SIZE;
+    NodeConfig cfg = container.getNodeConfig();
     if (cfg.getTransientCachePluginInfo() == null) {
-      // Still handle just having transientCacheSize defined in the body of solr.xml not in a transient handler clause.
-      this.cacheSize = cfg.getTransientCacheSize();
+      // Still handle just having transientCacheSize defined in the body of solr.xml
+      // not in a transient handler clause.
+      configuredCacheMaxSize = cfg.getTransientCacheSize();
     } else {
       NamedList<?> args = cfg.getTransientCachePluginInfo().initArgs;
       Object obj = args.get("transientCacheSize");
       if (obj != null) {
-        this.cacheSize = (int) obj;
+        configuredCacheMaxSize = (int) obj;
       }
     }
-
-    // it's possible for cache
-    if (cacheSize < 0) { // Trap old flag
-      cacheSize = Integer.MAX_VALUE;
+    if (configuredCacheMaxSize < 0) { // Trap old flag
+      configuredCacheMaxSize = Integer.MAX_VALUE;
     }
-
-    // Now don't allow ridiculous allocations here, if the size is > 1,000, we'll just deal with
-    // adding cores as they're opened. This blows up with the marker value of -1.
-    int actualCacheSize = Math.min(cacheSize, 1000);
-    log.info("Allocating transient cache for {} transient cores", actualCacheSize);
-    transientCores = new LinkedHashMap<>(actualCacheSize, 0.75f, true) {
-      @Override
-      protected boolean removeEldestEntry(Map.Entry<String, SolrCore> eldest) {
-        if (size() > cacheSize) {
-          SolrCore coreToClose = eldest.getValue();
-          if (log.isInfoEnabled()) {
-            log.info("Closing transient core [{}]", coreToClose.getName());
-          }
-          coreContainer.queueCoreToClose(coreToClose);
-          return true;
-        }
-        return false;
-      }
-    };
+    return configuredCacheMaxSize;
   }
 
-  
   @Override
   public Collection<SolrCore> prepareForShutdown() {
-    // Return a copy of the values
-
-    List<SolrCore> ret = new ArrayList<>(transientCores.values());
-    transientCores.clear();
+    // Return a copy of the values.
+    List<SolrCore> ret = new ArrayList<>(transientCores.asMap().values());
+    transientCores.invalidateAll();
+    transientCores.cleanUp();
     return ret;
   }
 
   @Override
-  public CoreContainer getContainer() { return this.coreContainer; }
+  public CoreContainer getContainer() {
+    return coreContainer;
+  }
 
   @Override
   public SolrCore addCore(String name, SolrCore core) {
-    return transientCores.put(name, core);
+    return transientCores.asMap().put(name, core);
   }
 
   @Override
@@ -121,38 +135,29 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
   
   @Override
   public Set<String> getLoadedCoreNames() {
-    return Collections.unmodifiableSet(transientCores.keySet());
+    return Collections.unmodifiableSet(transientCores.asMap().keySet());
   }
 
-  // Remove a core from the internal structures, presumably it 
-  // being closed. If the core is re-opened, it will be re-added by CoreContainer.
   @Override
   public SolrCore removeCore(String name) {
-    return transientCores.remove(name);
+    return transientCores.asMap().remove(name);
   }
 
-  // Get the core associated with the name. Return null if you don't want this core to be used.
   @Override
   public SolrCore getCore(String name) {
-    return transientCores.get(name);
+    return name == null ? null : transientCores.getIfPresent(name);
   }
 
   @Override
   public boolean containsCore(String name) {
-    return transientCores.containsKey(name);
+    return name != null && transientCores.asMap().containsKey(name);
   }
 
-  // These methods allow the implementation to maintain control over the core descriptors.
-
-
-  // This method will only be called during core discovery at startup.
   @Override
   public void addTransientDescriptor(String rawName, CoreDescriptor cd) {
     transientDescriptors.put(rawName, cd);
   }
 
-  // This method is used when opening cores and the like. If you want to change a core's descriptor, override this
-  // method and return the current core descriptor.
   @Override
   public CoreDescriptor getTransientDescriptor(String name) {
     return transientDescriptors.get(name);
@@ -168,11 +173,9 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
     return transientDescriptors.remove(name);
   }
 
-  // For custom implementations to communicate arbitrary information as necessary.
   @Override
   public int getStatus(String coreName) { return 0; } //no_op for default handler.
 
   @Override
   public void setStatus(String coreName, int status) {} //no_op for default handler.
-
 }
diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java b/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java
index 65d1f41..467ebb8 100644
--- a/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java
+++ b/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java
@@ -26,7 +26,6 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Properties;
 
@@ -155,8 +154,8 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
     CoreContainer cc = init();
     try {
 
-      TestLazyCores.checkInCores(cc, "core1");
-      TestLazyCores.checkNotInCores(cc, Arrays.asList("lazy1", "core2"));
+      TestLazyCores.checkLoadedCores(cc, "core1");
+      TestLazyCores.checkCoresNotLoaded(cc, "lazy1", "core2");
 
       // force loading of core2 and lazy1 by getting them from the CoreContainer
       try (SolrCore core1 = cc.getCore("core1");
@@ -174,7 +173,7 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
         assertEquals("solrconfig-minimal.xml", desc.getConfigName());
         assertEquals("schema-tiny.xml", desc.getSchemaName());
 
-        TestLazyCores.checkInCores(cc, "core1", "core2", "lazy1");
+        TestLazyCores.checkLoadedCores(cc, "core1", "core2", "lazy1");
         // Can we persist an existing core's properties?
 
         // Insure we can persist a new properties file if we want.
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 ff74d61..30bfdc0 100644
--- a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
+++ b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
@@ -22,7 +22,6 @@ import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -50,6 +49,9 @@ import org.junit.Test;
 
 public class TestLazyCores extends SolrTestCaseJ4 {
 
+  /** Transient core cache max size defined in the test solr.xml */
+  private static final int TRANSIENT_CORE_CACHE_MAX_SIZE = 4;
+
   private File solrHomeDirectory;
 
   @BeforeClass
@@ -65,7 +67,7 @@ public class TestLazyCores extends SolrTestCaseJ4 {
         CoreDescriptor.CORE_LOADONSTARTUP, loadOnStartup);
   }
 
-  private static CoresLocator testCores = new ReadOnlyCoresLocator() {
+  private static final CoresLocator testCores = new ReadOnlyCoresLocator() {
     @Override
     public List<CoreDescriptor> discover(CoreContainer cc) {
       return ImmutableList.of(
@@ -101,9 +103,9 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     try {
 
       // NOTE: This checks the initial state for loading, no need to do this elsewhere.
-      checkInCores(cc, "collection1", "collection2", "collection5");
-      checkNotInCores(cc, Arrays.asList("collection3", "collection4", "collection6", "collection7",
-          "collection8", "collection9"));
+      checkLoadedCores(cc, "collection1", "collection2", "collection5");
+      checkCoresNotLoaded(cc, "collection3", "collection4", "collection6", "collection7",
+          "collection8", "collection9");
 
       SolrCore core1 = cc.getCore("collection1");
       assertFalse("core1 should not be transient", core1.getCoreDescriptor().isTransient());
@@ -178,7 +180,7 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     CoreContainer cc = init();
     try {
       // Make sure Lazy4 isn't loaded. Should be loaded on the get
-      checkNotInCores(cc, Arrays.asList("collection4"));
+      checkCoresNotLoaded(cc, "collection4");
       SolrCore core4 = cc.getCore("collection4");
 
       checkSearch(core4);
@@ -191,7 +193,7 @@ public class TestLazyCores extends SolrTestCaseJ4 {
           , "//result[@numFound='0']"
       );
 
-      checkInCores(cc, "collection1", "collection2", "collection4", "collection5");
+      checkLoadedCores(cc, "collection1", "collection2", "collection4", "collection5");
 
       core4.close();
       collection1.close();
@@ -206,8 +208,8 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     try {
       // First check that all the cores that should be loaded at startup actually are.
 
-      checkInCores(cc, "collection1", "collection2", "collection5");
-      checkNotInCores(cc, Arrays.asList("collection3", "collection4", "collection6", "collection7", "collection8", "collection9"));
+      checkLoadedCores(cc, "collection1", "collection2", "collection5");
+      checkCoresNotLoaded(cc, "collection3", "collection4", "collection6", "collection7", "collection8", "collection9");
 
       // By putting these in non-alpha order, we're also checking that we're  not just seeing an artifact.
       SolrCore core1 = cc.getCore("collection1");
@@ -216,29 +218,34 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       SolrCore core2 = cc.getCore("collection2");
       SolrCore core5 = cc.getCore("collection5");
 
-      checkInCores(cc, "collection1", "collection2", "collection3", "collection4", "collection5");
-      checkNotInCores(cc, Arrays.asList("collection6", "collection7", "collection8", "collection9"));
+      checkLoadedCores(cc, "collection1", "collection2", "collection3", "collection4", "collection5");
+      checkCoresNotLoaded(cc, "collection6", "collection7", "collection8", "collection9");
 
       // map should be full up, add one more and verify
       SolrCore core6 = cc.getCore("collection6");
-      checkInCores(cc, "collection1", "collection2", "collection3", "collection4", "collection5",
+      checkLoadedCores(cc, "collection1", "collection2", "collection3", "collection4", "collection5",
           "collection6");
-      checkNotInCores(cc, Arrays.asList("collection7", "collection8", "collection9"));
+      checkCoresNotLoaded(cc, "collection7", "collection8", "collection9");
 
       SolrCore core7 = cc.getCore("collection7");
-      checkInCores(cc, "collection1", "collection2", "collection3", "collection4", "collection5",
+      checkLoadedCores(cc, "collection1", "collection2", "collection3", "collection4", "collection5",
           "collection6", "collection7");
-      checkNotInCores(cc, Arrays.asList("collection8", "collection9"));
+      checkCoresNotLoaded(cc, "collection8", "collection9");
 
       SolrCore core8 = cc.getCore("collection8");
-      checkInCores(cc, "collection1", "collection2", "collection4", "collection5", "collection6",
+      checkLoadedCores(cc, "collection1", "collection4", "collection5", "collection8");
+      checkSomeLoadedCores(cc, TRANSIENT_CORE_CACHE_MAX_SIZE, "collection2", "collection3", "collection6",
           "collection7", "collection8");
-      checkNotInCores(cc, Arrays.asList("collection3", "collection9"));
+      checkCoresNotLoaded(cc, "collection9");
+      checkSomeCoresNotLoaded(cc, 5 - TRANSIENT_CORE_CACHE_MAX_SIZE, "collection2", "collection3",
+          "collection6", "collection7");
 
       SolrCore core9 = cc.getCore("collection9");
-      checkInCores(cc, "collection1", "collection4", "collection5", "collection6", "collection7",
-          "collection8", "collection9");
-      checkNotInCores(cc, Arrays.asList("collection2", "collection3"));
+      checkLoadedCores(cc, "collection1", "collection4", "collection5", "collection9");
+      checkSomeLoadedCores(cc, TRANSIENT_CORE_CACHE_MAX_SIZE, "collection2", "collection3", "collection6",
+          "collection7", "collection8", "collection9");
+      checkSomeCoresNotLoaded(cc, 6 - TRANSIENT_CORE_CACHE_MAX_SIZE, "collection2", "collection3",
+          "collection6", "collection7", "collection8");
 
       // verify that getting metrics from an unloaded core doesn't cause exceptions (SOLR-12541)
       try (MetricsHandler handler = new MetricsHandler(h.getCoreContainer())) {
@@ -412,13 +419,15 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       final SolrCore c4 = cc.getCore("core4");
       final SolrCore c5 = cc.getCore("core5");
 
-      checkNotInCores(cc, Arrays.asList("core1", "collection2", "collection3", "collection4", "collection6"
-          , "collection7", "collection8", "collection9"));
+      checkCoresNotLoaded(cc, "collection2", "collection3", "collection4", "collection6",
+          "collection7", "collection8", "collection9");
+      checkSomeCoresNotLoaded(cc, 5 - TRANSIENT_CORE_CACHE_MAX_SIZE, "core1", "core2", "core3", "core4", "core5");
 
-      checkInCores(cc, "collection1", "collection5", "core2", "core3", "core4", "core5");
+      checkLoadedCores(cc, "collection1", "collection5");
+      checkSomeLoadedCores(cc, TRANSIENT_CORE_CACHE_MAX_SIZE, "core1", "core2", "core3", "core4", "core5");
 
       // While we're at it, a test for SOLR-5366, unloading transient core that's been unloaded b/c it's
-      // transient generates a "too many closes" errorl
+      // transient generates a "too many closes" error
 
       class TestThread extends Thread {
         
@@ -480,10 +489,10 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     
     try {
       // first, did the two good cores load successfully?
-      checkInCores(cc, "core1", "core2");
+      checkLoadedCores(cc, "core1", "core2");
 
       // Did the bad cores fail to load?
-      checkNotInCores(cc, Collections.emptyList(), Arrays.asList("badSchema1", "badSchema2", "badConfig1", "badConfig2"));
+      checkFailedCores(cc, "badSchema1", "badSchema2", "badConfig1", "badConfig2");
 
       //  Can we still search the "good" cores even though there were core init failures?
       SolrCore core1 = cc.getCore("core1");
@@ -524,7 +533,7 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       SolrCore bs2 = cc.getCore("badSchema2");
 
       // all the cores should be found in the list now.
-      checkInCores(cc, "core1", "core2", "badSchema1", "badSchema2", "badConfig1", "badConfig2");
+      checkLoadedCores(cc, "core1", "core2", "badSchema1", "badSchema2", "badConfig1", "badConfig2");
 
       // Did we clear out the errors by putting good files in place? And the cores that never were bad should be OK too.
       checkStatus(cc, true, "core1");
@@ -661,67 +670,77 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     }
   }
 
-  public static void checkNotInCores(CoreContainer cc, List<String> nameCheck) {
-    checkNotInCores(cc, nameCheck, Collections.emptyList());
+  public static void checkCoresNotLoaded(CoreContainer cc, String... coreNames) {
+    checkSomeCoresNotLoaded(cc, coreNames.length, coreNames);
   }
-  public static void checkNotInCores(CoreContainer cc, List<String> nameCheck, List<String> namesBad) {
-    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));
+
+  public static void checkSomeCoresNotLoaded(CoreContainer cc, int numNotLoaded, String... coreNames) {
+    Collection<String> loadedCoreNames = cc.getLoadedCoreNames();
+    List<String> notLoadedCoreNames = new ArrayList<>();
+    for (String coreName : coreNames) {
+      if (!loadedCoreNames.contains(coreName)) {
+        notLoadedCoreNames.add(coreName);
+      }
     }
-    
-    // There was a problem at one point exacerbated by the poor naming conventions. So parallel to loaded cores, there
-    // should be the ability to get the core _names_ that are loaded as well as all the core names _possible_
-    //
-    // the names above should only contain loaded core names. Every name in names should be in allNames, but none of 
-    // the names in nameCheck should be loaded and thus should not be in names.
-    
+    assertEquals("Expected " + numNotLoaded + " not loaded cores but found " + notLoadedCoreNames.size()
+            + ", coreNames=" + Arrays.asList(coreNames)
+            + ", notLoadedCoreNames=" + notLoadedCoreNames
+            + ", loadedCoreNames=" + loadedCoreNames,
+        numNotLoaded, notLoadedCoreNames.size());
+
+    // All transient cores are listed in allCoreNames.
+    Collection<String> allCoreNames = cc.getAllCoreNames();
+    for (String coreName : coreNames) {
+      assertTrue("Core " + coreName + " should have been found in the list of all known core names",
+          allCoreNames.contains(coreName));
+    }
+
+    checkCoreNamesAndDescriptors(cc);
+  }
+
+  private static void checkCoreNamesAndDescriptors(CoreContainer cc) {
     Collection<String> allNames = cc.getAllCoreNames();
-    // Every core that has not failed to load should be in coreDescriptors.
     List<CoreDescriptor> descriptors = cc.getCoreDescriptors();
 
+    // Every core that has not failed to load should be in coreDescriptors.
     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));
+      assertTrue("Each coreName should have a corresponding coreDescriptor", allNames.contains(desc.getName()));
     }
 
-    // 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",
+    // All loaded cores are in allNames.
+    for (String name : cc.getLoadedCoreNames()) {
+      assertTrue("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",
+  private static void checkFailedCores(CoreContainer cc, String... failedCoreNames) {
+    // Failed cores should not be in allCoreNames.
+    Collection<String> allNames = cc.getAllCoreNames();
+    for (String name : failedCoreNames) {
+      assertFalse("Failed core " + name + " should not have been found in the list of all possible core names",
           allNames.contains(name));
-      assertNull(cc.getCoreDescriptor(name));
-      assertFalse(cc.isLoaded(name));
     }
+  }
 
+  public static void checkLoadedCores(CoreContainer cc, String... coreNames) {
+    checkSomeLoadedCores(cc, coreNames.length, coreNames);
   }
 
-  public static void checkInCores(CoreContainer cc, String... nameCheck) {
-    Collection<String> loadedNames = cc.getLoadedCoreNames();
-    
-    assertEquals("There whould be exactly as many loaded cores as loaded names returned. ", 
-        loadedNames.size(), nameCheck.length);
-    
-    for (String name : nameCheck) {
-      assertTrue("core " + name + " was not found in the list of cores", loadedNames.contains(name));
+  public static void checkSomeLoadedCores(CoreContainer cc, int numLoaded, String... coreNames) {
+    Collection<String> loadedCoreNames = cc.getLoadedCoreNames();
+    List<String> loadedListedCoreNames = new ArrayList<>();
+    for (String coreName : coreNames) {
+      if (loadedCoreNames.contains(coreName)) {
+        loadedListedCoreNames.add(coreName);
+      }
     }
+    assertEquals("Expected " + numLoaded + " loaded cores but found " + loadedListedCoreNames.size()
+            + ", coreNames=" + Arrays.asList(coreNames)
+            + ", loadedListedCoreNames=" + loadedListedCoreNames
+            + ", loadedCoreNames=" + loadedCoreNames,
+        numLoaded, loadedListedCoreNames.size());
   }
 
   private void addLazy(SolrCore core, String... fieldValues) throws IOException {
@@ -806,13 +825,13 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     }
   }
 
-  // Insure that when a core is aged out of the transient cache, any uncommitted docs are preserved.
+  // Insure that when a core is evicted from the transient cache, any uncommitted docs are preserved.
   // Note, this needs FS-based indexes to persist!
   // Cores 2, 3, 6, 7, 8, 9 are transient
   @Test
   public void testNoCommit() throws Exception {
     CoreContainer cc = init();
-    String[] coreList = new String[]{
+    String[] transientCoreNames = new String[]{
         "collection2",
         "collection3",
         "collection6",
@@ -822,37 +841,44 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     };
     try {
       // First, go through all the transient cores and add some docs. DO NOT COMMIT!
-      // The aged-out core should commit the docs when it gets closed.
+      // The evicted core should commit the docs when it gets closed.
       List<SolrCore> openCores = new ArrayList<>();
-      for (String coreName : coreList) {
+      for (String coreName : transientCoreNames) {
         SolrCore core = cc.getCore(coreName);
         openCores.add(core);
         add10(core);
       }
       
-      // Just proving that some cores have been aged out.
-      checkNotInCores(cc, Arrays.asList("collection2", "collection3"));
+      // Just proving that some cores have been evicted to respect transient core cache max size.
+      checkSomeCoresNotLoaded(cc, transientCoreNames.length - TRANSIENT_CORE_CACHE_MAX_SIZE, transientCoreNames);
 
       // Close our get of all cores above.
       for (SolrCore core : openCores) core.close();
       openCores.clear();
       
-      // We still should have 6, 7, 8, 9 loaded, their reference counts have NOT dropped to zero 
-      checkInCores(cc, "collection1", "collection5",
-          "collection6", "collection7", "collection8", "collection9");
-
-      for (String coreName : coreList) {
-        // The point of this test is to insure that when cores are aged out and re-opened
-        // that the docs are there, so insure that the core we're testing is gone, gone, gone. 
-        checkNotInCores(cc, Arrays.asList(coreName));
-        
-        // Load the core up again.
-        SolrCore core = cc.getCore(coreName);
-        openCores.add(core);
-        
-        // Insure docs are still there.
-        check10(core);
+      // We still should have 4 transient cores loaded, their reference counts have NOT dropped to zero
+      checkLoadedCores(cc, "collection1", "collection5");
+      checkSomeLoadedCores(cc, TRANSIENT_CORE_CACHE_MAX_SIZE, transientCoreNames);
+
+      Collection<String> loadedCoreNames = cc.getLoadedCoreNames();
+      int notLoadedCoreCount = 0;
+      for (String coreName : transientCoreNames) {
+        // The point of this test is to insure that when cores are evicted and re-opened
+        // that the docs are there, so insure that the core we're testing is gone, gone, gone.
+        if (!loadedCoreNames.contains(coreName)) {
+          notLoadedCoreCount++;
+          checkCoresNotLoaded(cc, coreName);
+
+          // Load the core up again.
+          SolrCore core = cc.getCore(coreName);
+          openCores.add(core);
+          checkLoadedCores(cc, coreName);
+
+          // Insure docs are still there.
+          check10(core);
+        }
       }
+      assertEquals(transientCoreNames.length - TRANSIENT_CORE_CACHE_MAX_SIZE, notLoadedCoreCount);
       for (SolrCore core : openCores) core.close();
     } finally {
       cc.shutdown();