ben-manes commented on a change in pull request #147:
URL: https://github.com/apache/solr/pull/147#discussion_r639192576
##########
File path:
solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java
##########
@@ -18,103 +18,119 @@
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 CoreContainer coreContainer;
-
- protected final Map<String, CoreDescriptor> transientDescriptors = new
LinkedHashMap<>();
+ protected final CoreContainer coreContainer;
- //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
+ /**
+ * "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.
+ * <p>Note about Caffeine cache stats:
+ * Since we are using {@link Cache#asMap()},
+ * {@link com.github.benmanes.caffeine.cache.stats.CacheStats} are not
updated when we call
+ * any method of the Map view of the cache. This means
+ * {@link com.github.benmanes.caffeine.cache.stats.CacheStats} are not
accurate and should
+ * not be used.
+ */
+ protected final Cache<String, SolrCore> transientCores;
/**
- * @param container The enclosing CoreContainer. It allows us to access
everything we need.
+ * 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.
*/
- public TransientSolrCoreCacheDefault(final CoreContainer container) {
- this.coreContainer = container;
+ protected final Map<String, CoreDescriptor> transientDescriptors;
- 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 {
- @SuppressWarnings({"rawtypes"})
- NamedList args = cfg.getTransientCachePluginInfo().initArgs;
- Object obj = args.get("transientCacheSize");
- if (obj != null) {
- this.cacheSize = (int) obj;
- }
+ /**
+ * @param coreContainer The enclosing {@link CoreContainer}.
+ */
+ public TransientSolrCoreCacheDefault(CoreContainer coreContainer) {
+ this.coreContainer = coreContainer;
+
+ int cacheMaxSize = getConfiguredCacheMaxSize(coreContainer);
+ 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 {
@SuppressWarnings({"rawtypes"})
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
-
- @SuppressWarnings({"unchecked", "rawtypes"})
- 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);
Review comment:
The Cache interface is more opinionated regarding common expectations of
usage patterns, so `cache.put` returns `void` whereas `Map.put` returns the old
value. Generally a cache isn't inspected, e.g. there is no iterator of
memcached's entries. The `asMap` provides a more advanced escape hatch because
an opinionated interface doesn't match developer needs, so we just nudge
towards the usual best practices.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
