From ebfec751cad2f9698b0f15d071896fdcb2b6167c Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Wed, 17 Aug 2022 16:25:56 +1000 Subject: [PATCH 1/9] avoid reading schema xml from ZK, if it is already cached --- .../solr/cloud/ZkSolrResourceLoader.java | 15 +++ .../solr/schema/IndexSchemaFactory.java | 94 +++++++++++-------- .../schema/ManagedIndexSchemaFactory.java | 2 +- 3 files changed, 73 insertions(+), 38 deletions(-) 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..ad8082240a2 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.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 ZkSolrResourceLoader( configSetZkPath = ZkConfigSetService.CONFIGS_ZKNODE + "/" + configSet; } + public Pair 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/schema/IndexSchemaFactory.java b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java index ce287949322..d3867683813 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java @@ -24,11 +24,15 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import javax.xml.parsers.ParserConfigurationException; + +import com.google.common.base.Supplier; 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 String getSchemaResourceName(String cdResourceName) { 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()); @@ -110,44 +112,62 @@ public IndexSchema create( throw new SolrException(ErrorCode.SERVER_ERROR, msg, e); } } + public static ConfigSetService.ConfigResource getConfigResource( + ConfigSetService configSetService, + InputStream schemaInputStream, + SolrResourceLoader loader, + 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); + } + } @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 configCache = - (Map) - ((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; - } + public static VersionedConfig getFromCache(String name, + SolrResourceLoader loader, + com.google.common.base.Supplier objectCacheSupplier, + Supplier cfgLoader) { + if (loader instanceof ZkSolrResourceLoader) { + ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader) loader; + ObjectCache objectCache = objectCacheSupplier.get(); + if(objectCache == null) return cfgLoader.get(); + Map confCache = (Map) objectCache.computeIfAbsent( + ConfigSetService.ConfigResource.class.getName(), + k -> new ConcurrentHashMap<>()); + Pair 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 ManagedIndexSchema create( managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock()); - } catch (IOException e) { + } catch (Exception e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Error loading parsing schema", e); } if (shouldUpgrade) { From 579d8af2535dcd7b1cd5477fe0e6e180ae050a2f Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Wed, 17 Aug 2022 16:27:26 +1000 Subject: [PATCH 2/9] Revert "avoid reading schema xml from ZK, if it is already cached" This reverts commit ebfec751cad2f9698b0f15d071896fdcb2b6167c. wrong branch --- .../solr/cloud/ZkSolrResourceLoader.java | 15 --- .../solr/schema/IndexSchemaFactory.java | 94 ++++++++----------- .../schema/ManagedIndexSchemaFactory.java | 2 +- 3 files changed, 38 insertions(+), 73 deletions(-) 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 ad8082240a2..39efaaa06ce 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java @@ -24,7 +24,6 @@ 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; @@ -55,20 +54,6 @@ public ZkSolrResourceLoader( configSetZkPath = ZkConfigSetService.CONFIGS_ZKNODE + "/" + configSet; } - public Pair 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/schema/IndexSchemaFactory.java b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java index d3867683813..ce287949322 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java @@ -24,15 +24,11 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import javax.xml.parsers.ParserConfigurationException; - -import com.google.common.base.Supplier; 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; @@ -93,14 +89,16 @@ public String getSchemaResourceName(String cdResourceName) { 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, null, loader, resourceName), + getConfigResource(configSetService, schemaInputStream, loader, resourceName), config.luceneMatchVersion, loader, config.getSubstituteProperties()); @@ -112,62 +110,44 @@ public IndexSchema create( throw new SolrException(ErrorCode.SERVER_ERROR, msg, e); } } - public static ConfigSetService.ConfigResource getConfigResource( - ConfigSetService configSetService, - InputStream schemaInputStream, - SolrResourceLoader loader, - 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); - } - } @SuppressWarnings("unchecked") - public static VersionedConfig getFromCache(String name, - SolrResourceLoader loader, - com.google.common.base.Supplier objectCacheSupplier, - Supplier cfgLoader) { - if (loader instanceof ZkSolrResourceLoader) { - ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader) loader; - ObjectCache objectCache = objectCacheSupplier.get(); - if(objectCache == null) return cfgLoader.get(); - Map confCache = (Map) objectCache.computeIfAbsent( - ConfigSetService.ConfigResource.class.getName(), - k -> new ConcurrentHashMap<>()); - Pair 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()); + 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 configCache = + (Map) + ((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; + } } - } else { - // it's a file system loader, no caching necessary - return 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; + }; } + // 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 7dd6a81ebc9..7609149f14c 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 ManagedIndexSchema create( managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock()); - } catch (Exception e) { + } catch (IOException e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Error loading parsing schema", e); } if (shouldUpgrade) { From dc0f04fc773e1d39429d3e47ca45c2509c4313d6 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Wed, 17 Aug 2022 16:31:07 +1000 Subject: [PATCH 3/9] do not read schema file from ZK if it's already cached --- .../solr/cloud/ZkSolrResourceLoader.java | 15 +++ .../solr/schema/IndexSchemaFactory.java | 94 +++++++++++-------- .../schema/ManagedIndexSchemaFactory.java | 2 +- 3 files changed, 73 insertions(+), 38 deletions(-) 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..ad8082240a2 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.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 ZkSolrResourceLoader( configSetZkPath = ZkConfigSetService.CONFIGS_ZKNODE + "/" + configSet; } + public Pair 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/schema/IndexSchemaFactory.java b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java index ce287949322..d3867683813 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java @@ -24,11 +24,15 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import javax.xml.parsers.ParserConfigurationException; + +import com.google.common.base.Supplier; 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 String getSchemaResourceName(String cdResourceName) { 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()); @@ -110,44 +112,62 @@ public IndexSchema create( throw new SolrException(ErrorCode.SERVER_ERROR, msg, e); } } + public static ConfigSetService.ConfigResource getConfigResource( + ConfigSetService configSetService, + InputStream schemaInputStream, + SolrResourceLoader loader, + 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); + } + } @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 configCache = - (Map) - ((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; - } + public static VersionedConfig getFromCache(String name, + SolrResourceLoader loader, + com.google.common.base.Supplier objectCacheSupplier, + Supplier cfgLoader) { + if (loader instanceof ZkSolrResourceLoader) { + ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader) loader; + ObjectCache objectCache = objectCacheSupplier.get(); + if(objectCache == null) return cfgLoader.get(); + Map confCache = (Map) objectCache.computeIfAbsent( + ConfigSetService.ConfigResource.class.getName(), + k -> new ConcurrentHashMap<>()); + Pair 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 ManagedIndexSchema create( managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock()); - } catch (IOException e) { + } catch (Exception e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Error loading parsing schema", e); } if (shouldUpgrade) { From 0ca70e0cc647cf3e38c0e33dc9e22590d518a3a5 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Mon, 29 Aug 2022 13:07:52 +1000 Subject: [PATCH 4/9] cahe solrconfig.xml too --- .../java/org/apache/solr/core/SolrConfig.java | 65 +++++++------------ 1 file changed, 23 insertions(+), 42 deletions(-) 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..1173ab331aa 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.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 @@ private class ResourceProvider implements Function { 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,16 @@ private SolrConfig( this.substituteProperties = substitutableProperties; getOverlay(); // just in case it is not initialized // insist we have non-null substituteProperties; it might get overlaid - Map configCache = null; - if (loader.getCoreContainer() != null && loader.getCoreContainer().getObjectCache() != null) { - configCache = - (Map) - 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,19 +389,18 @@ private SolrConfig( } } - private void readXml( + private IndexSchemaFactory.VersionedConfig readXml( SolrResourceLoader loader, - String name, - Map 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)); - } - } + 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); + } + } private static final AtomicBoolean versionWarningAlreadyLogged = new AtomicBoolean(false); From 396b989c576c0a452bbc132d0db4d00c7951aabb Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Mon, 29 Aug 2022 13:45:30 +1000 Subject: [PATCH 5/9] adde tests --- .../solr/schema/IndexSchemaFactory.java | 13 +++++++-- .../api/collections/TestCollectionAPI.java | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) 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 d3867683813..64245ee7f3f 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java @@ -23,6 +23,7 @@ import java.lang.invoke.MethodHandles; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Consumer; import javax.xml.parsers.ParserConfigurationException; import com.google.common.base.Supplier; @@ -142,11 +143,19 @@ private static VersionedConfig loadConfig(InputStream schemaInputStream, SolrRes } } + + //for testing purposes + public static volatile Consumer CACHE_MISS_LISTENER = null; @SuppressWarnings("unchecked") public static VersionedConfig getFromCache(String name, SolrResourceLoader loader, - com.google.common.base.Supplier objectCacheSupplier, - Supplier cfgLoader) { + Supplier objectCacheSupplier, + Supplier c) { + Supplier cfgLoader = () -> { + Consumer listener = CACHE_MISS_LISTENER; + if (listener != null) listener.accept(name); + return c.get(); + }; if (loader instanceof ZkSolrResourceLoader) { ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader) loader; ObjectCache objectCache = objectCacheSupplier.get(); 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..147adca28e7 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,8 @@ 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 +37,7 @@ 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 +53,7 @@ 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 +1283,28 @@ public void testRecreateCollectionAfterFailure() throws Exception { 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(); + } + + } } From 89bb5e84c76d6ea76ecd4ccd588ea489945dd07c Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Mon, 29 Aug 2022 13:48:37 +1000 Subject: [PATCH 6/9] formatting --- .../solr/cloud/ZkSolrResourceLoader.java | 2 +- .../java/org/apache/solr/core/SolrConfig.java | 28 ++++--- .../solr/schema/IndexSchemaFactory.java | 81 ++++++++++--------- .../api/collections/TestCollectionAPI.java | 17 ++-- 4 files changed, 68 insertions(+), 60 deletions(-) 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 ad8082240a2..4aa5990e0ea 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java @@ -59,7 +59,7 @@ public Pair getZkResourceInfo(String resource) { String file = (".".equals(resource)) ? configSetZkPath : configSetZkPath + "/" + resource; try { Stat stat = zkController.getZkClient().exists(file, null, true); - if(stat != null) { + if (stat != null) { return new Pair<>(file, stat.getVersion()); } else { return null; 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 1173ab331aa..af3d994a5c3 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrConfig.java +++ b/solr/core/src/java/org/apache/solr/core/SolrConfig.java @@ -213,7 +213,10 @@ private SolrConfig( getOverlay(); // just in case it is not initialized // insist we have non-null substituteProperties; it might get overlaid - IndexSchemaFactory.VersionedConfig cfg = IndexSchemaFactory.getFromCache(name, loader, + IndexSchemaFactory.VersionedConfig cfg = + IndexSchemaFactory.getFromCache( + name, + loader, () -> { if (loader.getCoreContainer() == null) return null; return loader.getCoreContainer().getObjectCache(); @@ -389,18 +392,17 @@ private SolrConfig( } } - 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); - } - } + 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); + } + } private static final AtomicBoolean versionWarningAlreadyLogged = new AtomicBoolean(false); 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 64245ee7f3f..5fe3e57fe24 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java @@ -18,6 +18,7 @@ import static org.apache.solr.schema.IndexSchema.SCHEMA; +import com.google.common.base.Supplier; import java.io.IOException; import java.io.InputStream; import java.lang.invoke.MethodHandles; @@ -25,8 +26,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import javax.xml.parsers.ParserConfigurationException; - -import com.google.common.base.Supplier; import org.apache.solr.cloud.ZkConfigSetService; import org.apache.solr.cloud.ZkSolrResourceLoader; import org.apache.solr.common.ConfigNode; @@ -113,58 +112,66 @@ public IndexSchema create( throw new SolrException(ErrorCode.SERVER_ERROR, msg, e); } } + public static ConfigSetService.ConfigResource getConfigResource( - ConfigSetService configSetService, - InputStream schemaInputStream, - SolrResourceLoader loader, - String name) { - return () -> getFromCache(name, loader, - () -> { - if(!(configSetService instanceof ZkConfigSetService)) return null; - return ((ZkConfigSetService) configSetService) + ConfigSetService configSetService, + InputStream schemaInputStream, + SolrResourceLoader loader, + String name) { + return () -> + getFromCache( + name, + loader, + () -> { + if (!(configSetService instanceof ZkConfigSetService)) return null; + return ((ZkConfigSetService) configSetService) .getSolrCloudManager() .getObjectCache(); - }, - () -> loadConfig(schemaInputStream, loader, name)).data; + }, + () -> loadConfig(schemaInputStream, loader, name)) + .data; } - private static VersionedConfig loadConfig(InputStream schemaInputStream, SolrResourceLoader loader, String name) { + + 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; + 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 + // for testing purposes public static volatile Consumer CACHE_MISS_LISTENER = null; + @SuppressWarnings("unchecked") - public static VersionedConfig getFromCache(String name, - SolrResourceLoader loader, - Supplier objectCacheSupplier, - Supplier c) { - Supplier cfgLoader = () -> { - Consumer listener = CACHE_MISS_LISTENER; - if (listener != null) listener.accept(name); - return c.get(); - }; + public static VersionedConfig getFromCache( + String name, + SolrResourceLoader loader, + Supplier objectCacheSupplier, + Supplier c) { + Supplier cfgLoader = + () -> { + Consumer listener = CACHE_MISS_LISTENER; + if (listener != null) 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 confCache = (Map) objectCache.computeIfAbsent( - ConfigSetService.ConfigResource.class.getName(), - k -> new ConcurrentHashMap<>()); + if (objectCache == null) return cfgLoader.get(); + Map confCache = + (Map) + objectCache.computeIfAbsent( + ConfigSetService.ConfigResource.class.getName(), k -> new ConcurrentHashMap<>()); Pair res = zkLoader.getZkResourceInfo(name); - if(res == null) return cfgLoader.get(); + if (res == null) return cfgLoader.get(); VersionedConfig result = null; result = confCache.computeIfAbsent(res.first(), k -> cfgLoader.get()); if (result.version == res.second()) { 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 147adca28e7..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 @@ -26,7 +26,6 @@ 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; @@ -1287,24 +1286,24 @@ public void testRecreateCollectionAfterFailure() throws Exception { @Test public void testConfigCaching() throws Exception { String COLL = "cfg_cache_test"; - MiniSolrCloudCluster cl = new MiniSolrCloudCluster.Builder(2, createTempDir()) + 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()); + 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(); } - } } From 35e514a2b13cf64785fb256c009111621179afe2 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Mon, 29 Aug 2022 15:39:15 +1000 Subject: [PATCH 7/9] formatting --- .../src/java/org/apache/solr/schema/IndexSchemaFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5fe3e57fe24..b45864cf152 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java @@ -18,13 +18,13 @@ import static org.apache.solr.schema.IndexSchema.SCHEMA; -import com.google.common.base.Supplier; import java.io.IOException; 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; From 78f75d6c6313ea9f1fc2120827986877059993ae Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Wed, 31 Aug 2022 22:43:00 +1000 Subject: [PATCH 8/9] refactor --- .../org/apache/solr/schema/IndexSchemaFactory.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) 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 b45864cf152..f3690e9aee1 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java @@ -156,12 +156,15 @@ public static VersionedConfig getFromCache( SolrResourceLoader loader, Supplier objectCacheSupplier, Supplier c) { + Consumer listener = CACHE_MISS_LISTENER; Supplier cfgLoader = - () -> { - Consumer listener = CACHE_MISS_LISTENER; - if (listener != null) listener.accept(name); - return c.get(); - }; + listener == null + ? c + : () -> { + listener.accept(name); + return c.get(); + }; + if (loader instanceof ZkSolrResourceLoader) { ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader) loader; ObjectCache objectCache = objectCacheSupplier.get(); From 92590a961f2858ce1e258ccaba18322b6966fb45 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Wed, 31 Aug 2022 22:47:44 +1000 Subject: [PATCH 9/9] CHANGES.txt --- solr/CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f51145b8cce..35321ca4f7b 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -80,6 +80,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)