You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by no...@apache.org on 2022/08/31 13:17:01 UTC
[solr] branch main updated: SOLR-16336: avoid fetching solrconfig.xml & schema.xml for already cached schema and config (#987)
This is an automated email from the ASF dual-hosted git repository.
noble 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 d7a4c6fd0fb SOLR-16336: avoid fetching solrconfig.xml & schema.xml for already cached schema and config (#987)
d7a4c6fd0fb is described below
commit d7a4c6fd0fb057caab30915cb08c33c3b1aa353f
Author: Noble Paul <no...@users.noreply.github.com>
AuthorDate: Wed Aug 31 23:16:55 2022 +1000
SOLR-16336: avoid fetching solrconfig.xml & schema.xml for already cached schema and config (#987)
---
solr/CHANGES.txt | 2 +
.../apache/solr/cloud/ZkSolrResourceLoader.java | 15 +++
.../src/java/org/apache/solr/core/SolrConfig.java | 65 +++++--------
.../org/apache/solr/schema/IndexSchemaFactory.java | 105 ++++++++++++++-------
.../solr/schema/ManagedIndexSchemaFactory.java | 2 +-
.../cloud/api/collections/TestCollectionAPI.java | 27 ++++++
6 files changed, 141 insertions(+), 75 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 553a7816b66..5a8bd7a7271 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -85,6 +85,8 @@ Optimizations
* SOLR-9359: Enable warming queries to be managed using Config API (Andy Webb, Eric Pugh)
+* SOLR-16336: avoid fetching solrconfig.xml & schema.xml for already cached schema and config (noble)
+
Bug Fixes
---------------------
* SOLR-15918: Skip repetitive parent znode creation on config set upload (Mike Drob)
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
index 39efaaa06ce..4aa5990e0ea 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
@@ -24,6 +24,7 @@ import java.lang.invoke.MethodHandles;
import java.nio.file.Path;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.ZooKeeperException;
+import org.apache.solr.common.util.Pair;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrResourceLoader;
import org.apache.solr.core.SolrResourceNotFoundException;
@@ -54,6 +55,20 @@ public class ZkSolrResourceLoader extends SolrResourceLoader {
configSetZkPath = ZkConfigSetService.CONFIGS_ZKNODE + "/" + configSet;
}
+ public Pair<String, Integer> getZkResourceInfo(String resource) {
+ String file = (".".equals(resource)) ? configSetZkPath : configSetZkPath + "/" + resource;
+ try {
+ Stat stat = zkController.getZkClient().exists(file, null, true);
+ if (stat != null) {
+ return new Pair<>(file, stat.getVersion());
+ } else {
+ return null;
+ }
+ } catch (Exception e) {
+ return null;
+ }
+ }
+
/**
* Opens any resource by its name. By default, this will look in multiple locations to load the
* resource: $configDir/$resource from ZooKeeper. It will look for it in any jar accessible
diff --git a/solr/core/src/java/org/apache/solr/core/SolrConfig.java b/solr/core/src/java/org/apache/solr/core/SolrConfig.java
index f5177c869b5..af3d994a5c3 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrConfig.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrConfig.java
@@ -49,7 +49,6 @@ import java.util.Objects;
import java.util.Properties;
import java.util.Set;
import java.util.UUID;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -173,8 +172,8 @@ public class SolrConfig implements MapSerializable {
InputStream in;
String fileName;
- ResourceProvider(InputStream in) {
- this.in = in;
+ ResourceProvider(SolrResourceLoader loader, String res) throws IOException {
+ this.in = loader.openResource(res);
if (in instanceof ZkSolrResourceLoader.ZkByteArrayInputStream) {
ZkSolrResourceLoader.ZkByteArrayInputStream zkin =
(ZkSolrResourceLoader.ZkByteArrayInputStream) in;
@@ -213,33 +212,19 @@ public class SolrConfig implements MapSerializable {
this.substituteProperties = substitutableProperties;
getOverlay(); // just in case it is not initialized
// insist we have non-null substituteProperties; it might get overlaid
- Map<String, IndexSchemaFactory.VersionedConfig> configCache = null;
- if (loader.getCoreContainer() != null && loader.getCoreContainer().getObjectCache() != null) {
- configCache =
- (Map<String, IndexSchemaFactory.VersionedConfig>)
- loader
- .getCoreContainer()
- .getObjectCache()
- .computeIfAbsent(
- ConfigSetService.ConfigResource.class.getName(),
- s -> new ConcurrentHashMap<>());
- ResourceProvider rp = new ResourceProvider(loader.openResource(name));
- IndexSchemaFactory.VersionedConfig cfg =
- rp.fileName == null ? null : configCache.get(rp.fileName);
- if (cfg != null) {
- if (rp.hash != -1) {
- if (rp.hash == cfg.version) {
- log.debug("LOADED_FROM_CACHE");
- root = cfg.data;
- } else {
- readXml(loader, name, configCache, rp);
- }
- }
- }
- }
- if (root == null) {
- readXml(loader, name, configCache, new ResourceProvider(loader.openResource(name)));
- }
+
+ IndexSchemaFactory.VersionedConfig cfg =
+ IndexSchemaFactory.getFromCache(
+ name,
+ loader,
+ () -> {
+ if (loader.getCoreContainer() == null) return null;
+ return loader.getCoreContainer().getObjectCache();
+ },
+ () -> readXml(loader, name));
+ this.root = cfg.data;
+ this.znodeVersion = cfg.version;
+
ConfigNode.SUBSTITUTES.set(
key -> {
if (substitutableProperties != null && substitutableProperties.containsKey(key)) {
@@ -407,17 +392,15 @@ public class SolrConfig implements MapSerializable {
}
}
- private void readXml(
- SolrResourceLoader loader,
- String name,
- Map<String, IndexSchemaFactory.VersionedConfig> configCache,
- ResourceProvider rp)
- throws IOException {
- XmlConfigFile xml = new XmlConfigFile(loader, rp, name, null, "/config/", null);
- root = new DataConfigNode(new DOMConfigNode(xml.getDocument().getDocumentElement()));
- this.znodeVersion = rp.zkVersion;
- if (configCache != null && rp.fileName != null) {
- configCache.put(rp.fileName, new IndexSchemaFactory.VersionedConfig(rp.hash, root));
+ private IndexSchemaFactory.VersionedConfig readXml(SolrResourceLoader loader, String name) {
+ try {
+ ResourceProvider rp = new ResourceProvider(loader, name);
+ XmlConfigFile xml = new XmlConfigFile(loader, rp, name, null, "/config/", null);
+ return new IndexSchemaFactory.VersionedConfig(
+ rp.zkVersion,
+ new DataConfigNode(new DOMConfigNode(xml.getDocument().getDocumentElement())));
+ } catch (IOException e) {
+ throw new SolrException(ErrorCode.SERVER_ERROR, e);
}
}
diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java
index ce287949322..f3690e9aee1 100644
--- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java
+++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java
@@ -23,12 +23,16 @@ import java.io.InputStream;
import java.lang.invoke.MethodHandles;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
import javax.xml.parsers.ParserConfigurationException;
import org.apache.solr.cloud.ZkConfigSetService;
import org.apache.solr.cloud.ZkSolrResourceLoader;
import org.apache.solr.common.ConfigNode;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.util.ObjectCache;
+import org.apache.solr.common.util.Pair;
import org.apache.solr.core.ConfigSetService;
import org.apache.solr.core.PluginInfo;
import org.apache.solr.core.SolrConfig;
@@ -89,16 +93,14 @@ public abstract class IndexSchemaFactory implements NamedListInitializedPlugin {
public IndexSchema create(
String resourceName, SolrConfig config, ConfigSetService configSetService) {
SolrResourceLoader loader = config.getResourceLoader();
- InputStream schemaInputStream = null;
if (null == resourceName) {
resourceName = IndexSchema.DEFAULT_SCHEMA_FILE;
}
try {
- schemaInputStream = loader.openResource(resourceName);
return new IndexSchema(
resourceName,
- getConfigResource(configSetService, schemaInputStream, loader, resourceName),
+ getConfigResource(configSetService, null, loader, resourceName),
config.luceneMatchVersion,
loader,
config.getSubstituteProperties());
@@ -111,43 +113,80 @@ public abstract class IndexSchemaFactory implements NamedListInitializedPlugin {
}
}
- @SuppressWarnings("unchecked")
public static ConfigSetService.ConfigResource getConfigResource(
ConfigSetService configSetService,
InputStream schemaInputStream,
SolrResourceLoader loader,
- String name)
- throws IOException {
- if (configSetService instanceof ZkConfigSetService
- && schemaInputStream instanceof ZkSolrResourceLoader.ZkByteArrayInputStream) {
- ZkSolrResourceLoader.ZkByteArrayInputStream is =
- (ZkSolrResourceLoader.ZkByteArrayInputStream) schemaInputStream;
- Map<String, VersionedConfig> configCache =
+ String name) {
+ return () ->
+ getFromCache(
+ name,
+ loader,
+ () -> {
+ if (!(configSetService instanceof ZkConfigSetService)) return null;
+ return ((ZkConfigSetService) configSetService)
+ .getSolrCloudManager()
+ .getObjectCache();
+ },
+ () -> loadConfig(schemaInputStream, loader, name))
+ .data;
+ }
+
+ private static VersionedConfig loadConfig(
+ InputStream schemaInputStream, SolrResourceLoader loader, String name) {
+ try {
+ InputStream is = (schemaInputStream == null ? loader.openResource(name) : schemaInputStream);
+ ConfigNode node = getParsedSchema(is, loader, name);
+ int version =
+ is instanceof ZkSolrResourceLoader.ZkByteArrayInputStream
+ ? ((ZkSolrResourceLoader.ZkByteArrayInputStream) is).getStat().getVersion()
+ : 0;
+ return new VersionedConfig(version, node);
+ } catch (Exception e) {
+ throw new SolrException(ErrorCode.SERVER_ERROR, "Error fetching schema", e);
+ }
+ }
+
+ // for testing purposes
+ public static volatile Consumer<String> CACHE_MISS_LISTENER = null;
+
+ @SuppressWarnings("unchecked")
+ public static VersionedConfig getFromCache(
+ String name,
+ SolrResourceLoader loader,
+ Supplier<ObjectCache> objectCacheSupplier,
+ Supplier<VersionedConfig> c) {
+ Consumer<String> listener = CACHE_MISS_LISTENER;
+ Supplier<VersionedConfig> cfgLoader =
+ listener == null
+ ? c
+ : () -> {
+ listener.accept(name);
+ return c.get();
+ };
+
+ if (loader instanceof ZkSolrResourceLoader) {
+ ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader) loader;
+ ObjectCache objectCache = objectCacheSupplier.get();
+ if (objectCache == null) return cfgLoader.get();
+ Map<String, VersionedConfig> confCache =
(Map<String, VersionedConfig>)
- ((ZkConfigSetService) configSetService)
- .getSolrCloudManager()
- .getObjectCache()
- .computeIfAbsent(
- ConfigSetService.ConfigResource.class.getName(),
- s -> new ConcurrentHashMap<>());
- VersionedConfig cached = configCache.get(is.fileName);
- if (cached != null) {
- if (cached.version != is.getStat().getVersion()) {
- configCache.remove(is.fileName); // this is stale. remove from cache
- } else {
- return () -> cached.data;
- }
+ objectCache.computeIfAbsent(
+ ConfigSetService.ConfigResource.class.getName(), k -> new ConcurrentHashMap<>());
+ Pair<String, Integer> res = zkLoader.getZkResourceInfo(name);
+ if (res == null) return cfgLoader.get();
+ VersionedConfig result = null;
+ result = confCache.computeIfAbsent(res.first(), k -> cfgLoader.get());
+ if (result.version == res.second()) {
+ return result;
+ } else {
+ confCache.remove(res.first());
+ return confCache.computeIfAbsent(res.first(), k -> cfgLoader.get());
}
- return () -> {
- ConfigNode data =
- getParsedSchema(
- schemaInputStream, loader, name); // either missing or stale. create a new one
- configCache.put(is.fileName, new VersionedConfig(is.getStat().getVersion(), data));
- return data;
- };
+ } else {
+ // it's a file system loader, no caching necessary
+ return cfgLoader.get();
}
- // this is not cacheable as it does not come from ZK
- return () -> getParsedSchema(schemaInputStream, loader, name);
}
public static ConfigNode getParsedSchema(InputStream is, SolrResourceLoader loader, String name)
diff --git a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
index 7609149f14c..7dd6a81ebc9 100644
--- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
+++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
@@ -277,7 +277,7 @@ public class ManagedIndexSchemaFactory extends IndexSchemaFactory implements Sol
managedSchemaResourceName,
schemaZkVersion,
getSchemaUpdateLock());
- } catch (IOException e) {
+ } catch (Exception e) {
throw new SolrException(ErrorCode.SERVER_ERROR, "Error loading parsing schema", e);
}
if (shouldUpgrade) {
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
index f7e1667dc3d..96c64a2234f 100644
--- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
@@ -25,6 +25,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.LongAdder;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.SolrServerException;
@@ -35,6 +36,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.V2Request;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
+import org.apache.solr.cloud.MiniSolrCloudCluster;
import org.apache.solr.cloud.ZkConfigSetService;
import org.apache.solr.cloud.ZkTestServer;
import org.apache.solr.common.SolrException;
@@ -50,6 +52,7 @@ import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.ShardParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.Utils;
+import org.apache.solr.schema.IndexSchemaFactory;
import org.apache.zookeeper.KeeperException;
import org.junit.Test;
@@ -1279,4 +1282,28 @@ public class TestCollectionAPI extends ReplicaPropertiesBase {
assertSame(0, rsp.getStatus());
}
}
+
+ @Test
+ public void testConfigCaching() throws Exception {
+ String COLL = "cfg_cache_test";
+ MiniSolrCloudCluster cl =
+ new MiniSolrCloudCluster.Builder(2, createTempDir())
+ .addConfig("conf", configset("cloud-minimal"))
+ .configure();
+
+ try {
+ LongAdder schemaMisses = new LongAdder();
+ LongAdder configMisses = new LongAdder();
+ IndexSchemaFactory.CACHE_MISS_LISTENER =
+ s -> {
+ if ("schema.xml".equals(s)) schemaMisses.increment();
+ if ("solrconfig.xml".equals(s)) configMisses.increment();
+ };
+ CollectionAdminRequest.createCollection(COLL, "conf", 5, 1).process(cl.getSolrClient());
+ assertEquals(2, schemaMisses.longValue());
+ assertEquals(2, configMisses.longValue());
+ } finally {
+ cl.shutdown();
+ }
+ }
}