diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index e9661ffde88..531ba9b4196 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -126,6 +126,9 @@ Bug Fixes * SOLR-13244: Admin UI Nodes view fails and is empty when a node is temporarily down (janhoy) +* SOLR-13253: IndexSchema.getResourceLoader was being used to load non-schema things, which can be a memory leak if + "shareSchema" and other circumstances occur. Furthermore it's reference to SolrConfig was removed. (David Smiley) + Improvements ---------------------- diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 93bd69001f6..c86ceb4d4e0 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -1388,7 +1388,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab final PluginInfo info = solrConfig.getPluginInfo(CodecFactory.class.getName()); final CodecFactory factory; if (info != null) { - factory = schema.getResourceLoader().newInstance(info.className, CodecFactory.class); + factory = resourceLoader.newInstance(info.className, CodecFactory.class); factory.init(info.initArgs); } else { factory = new CodecFactory() { diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java index ce623e6cb1f..8042e80f71c 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -33,6 +33,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; @@ -59,9 +60,9 @@ import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.Pair; import org.apache.solr.common.util.SimpleOrderedMap; -import org.apache.solr.core.XmlConfigFile; -import org.apache.solr.core.SolrConfig; +import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrResourceLoader; +import org.apache.solr.core.XmlConfigFile; import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.response.SchemaXmlWriter; import org.apache.solr.response.SolrQueryResponse; @@ -126,9 +127,9 @@ public class IndexSchema { private static final String XPATH_OR = " | "; private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - protected final SolrConfig solrConfig; protected String resourceName; protected String name; + protected final Version luceneVersion; protected float version; protected final SolrResourceLoader loader; @@ -161,20 +162,16 @@ public class IndexSchema { */ protected Map copyFieldTargetCounts = new HashMap<>(); - /** + /** * Constructs a schema using the specified resource name and stream. * @see SolrResourceLoader#openSchema * By default, this follows the normal config path directory searching rules. * @see SolrResourceLoader#openResource */ - public IndexSchema(SolrConfig solrConfig, String name, InputSource is) { - assert null != solrConfig : "SolrConfig should never be null"; - assert null != name : "schema resource name should never be null"; - assert null != is : "schema InputSource should never be null"; + public IndexSchema(String name, InputSource is, Version luceneVersion, SolrResourceLoader resourceLoader) { + this(luceneVersion, resourceLoader); - this.solrConfig = solrConfig; - this.resourceName = name; - loader = solrConfig.getResourceLoader(); + this.resourceName = Objects.requireNonNull(name); try { readSchema(is); loader.inform(loader); @@ -183,10 +180,20 @@ public class IndexSchema { } } + protected IndexSchema(Version luceneVersion, SolrResourceLoader loader) { + this.luceneVersion = Objects.requireNonNull(luceneVersion); + this.loader = loader; + } + /** + * The resource loader to be used to load components related to the schema when the schema is loading + * / initialising. + * It should not be used for any other purpose or time; + * consider {@link SolrCore#getResourceLoader()} instead. * @since solr 1.4 */ public SolrResourceLoader getResourceLoader() { + //TODO consider asserting the schema has not finished loading somehow? return loader; } @@ -207,7 +214,7 @@ public class IndexSchema { /** The Default Lucene Match Version for this IndexSchema */ public Version getDefaultLuceneMatchVersion() { - return solrConfig.luceneMatchVersion; + return luceneVersion; } public float getVersion() { @@ -443,6 +450,7 @@ public class IndexSchema { } protected void readSchema(InputSource is) { + assert null != is : "schema InputSource should never be null"; try { // pass the config resource loader to avoid building an empty one for no reason: // in the current case though, the stream is valid so we wont load the resource by name @@ -1595,11 +1603,6 @@ public class IndexSchema { return copyFieldProperties; } - protected IndexSchema(final SolrConfig solrConfig, final SolrResourceLoader loader) { - this.solrConfig = solrConfig; - this.loader = loader; - } - /** * Copies this schema, adds the given field to the copy * Requires synchronizing on the object returned by 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 fda4868c27e..4234ba1d844 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchemaFactory.java @@ -16,10 +16,14 @@ */ package org.apache.solr.schema; +import java.io.File; +import java.io.InputStream; +import java.lang.invoke.MethodHandles; + import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.core.PluginInfo; -import org.apache.solr.core.SolrConfig; +import org.apache.solr.core.SolrConfig; import org.apache.solr.core.SolrResourceLoader; import org.apache.solr.util.SystemIdResolver; import org.apache.solr.util.plugin.NamedListInitializedPlugin; @@ -27,10 +31,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xml.sax.InputSource; -import java.io.File; -import java.io.InputStream; -import java.lang.invoke.MethodHandles; - /** Base class for factories for IndexSchema implementations */ public abstract class IndexSchemaFactory implements NamedListInitializedPlugin { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -53,7 +53,7 @@ public abstract class IndexSchemaFactory implements NamedListInitializedPlugin { } InputSource inputSource = new InputSource(schemaInputStream); inputSource.setSystemId(SystemIdResolver.createSystemIdFromResourceName(resourceName)); - IndexSchema schema = new IndexSchema(config, resourceName, inputSource); + IndexSchema schema = new IndexSchema(resourceName, inputSource, config.luceneMatchVersion, loader); return schema; } diff --git a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java index 0314ad9311c..c7fbf276be5 100644 --- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java @@ -44,6 +44,7 @@ import org.apache.lucene.analysis.util.CharFilterFactory; import org.apache.lucene.analysis.util.ResourceLoaderAware; import org.apache.lucene.analysis.util.TokenFilterFactory; import org.apache.lucene.analysis.util.TokenizerFactory; +import org.apache.lucene.util.Version; import org.apache.solr.analysis.TokenizerChain; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; @@ -100,9 +101,8 @@ public final class ManagedIndexSchema extends IndexSchema { * @see org.apache.solr.core.SolrResourceLoader#openResource */ ManagedIndexSchema(SolrConfig solrConfig, String name, InputSource is, boolean isMutable, - String managedSchemaResourceName, int schemaZkVersion, Object schemaUpdateLock) - throws KeeperException, InterruptedException { - super(solrConfig, name, is); + String managedSchemaResourceName, int schemaZkVersion, Object schemaUpdateLock) { + super(name, is, solrConfig.luceneMatchVersion, solrConfig.getResourceLoader()); this.isMutable = isMutable; this.managedSchemaResourceName = managedSchemaResourceName; this.schemaZkVersion = schemaZkVersion; @@ -1320,10 +1320,9 @@ public final class ManagedIndexSchema extends IndexSchema { } } - private ManagedIndexSchema(final SolrConfig solrConfig, final SolrResourceLoader loader, boolean isMutable, - String managedSchemaResourceName, int schemaZkVersion, Object schemaUpdateLock) - throws KeeperException, InterruptedException { - super(solrConfig, loader); + private ManagedIndexSchema(Version luceneVersion, SolrResourceLoader loader, boolean isMutable, + String managedSchemaResourceName, int schemaZkVersion, Object schemaUpdateLock) { + super(luceneVersion, loader); this.isMutable = isMutable; this.managedSchemaResourceName = managedSchemaResourceName; this.schemaZkVersion = schemaZkVersion; @@ -1340,22 +1339,9 @@ public final class ManagedIndexSchema extends IndexSchema { * @return A shallow copy of this schema */ ManagedIndexSchema shallowCopy(boolean includeFieldDataStructures) { - ManagedIndexSchema newSchema = null; - try { - newSchema = new ManagedIndexSchema - (solrConfig, loader, isMutable, managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock()); - } catch (KeeperException e) { - final String msg = "Error instantiating ManagedIndexSchema"; - log.error(msg, e); - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, msg, e); - } catch (InterruptedException e) { - // Restore the interrupted status - Thread.currentThread().interrupt(); - log.warn("", e); - } + ManagedIndexSchema newSchema = new ManagedIndexSchema + (luceneVersion, loader, isMutable, managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock()); - assert newSchema != null; - newSchema.name = name; newSchema.version = version; newSchema.similarity = similarity; 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 e433dc4d650..72c3d6f087a 100644 --- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java @@ -168,19 +168,8 @@ public class ManagedIndexSchemaFactory extends IndexSchemaFactory implements Sol } InputSource inputSource = new InputSource(schemaInputStream); inputSource.setSystemId(SystemIdResolver.createSystemIdFromResourceName(loadedResource)); - try { - schema = new ManagedIndexSchema(config, loadedResource, inputSource, isMutable, - managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock()); - } catch (KeeperException e) { - final String msg = "Error instantiating ManagedIndexSchema"; - log.error(msg, e); - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, msg, e); - } catch (InterruptedException e) { - // Restore the interrupted status - Thread.currentThread().interrupt(); - log.warn("", e); - } - + schema = new ManagedIndexSchema(config, loadedResource, inputSource, isMutable, + managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock()); if (shouldUpgrade) { // Persist the managed schema if it doesn't already exist upgradeToManagedSchema(); diff --git a/solr/core/src/java/org/apache/solr/schema/SchemaManager.java b/solr/core/src/java/org/apache/solr/schema/SchemaManager.java index 217c726fd1e..afc3b04fe89 100644 --- a/solr/core/src/java/org/apache/solr/schema/SchemaManager.java +++ b/solr/core/src/java/org/apache/solr/schema/SchemaManager.java @@ -159,16 +159,16 @@ public class SchemaManager { } private void waitForOtherReplicasToUpdate(TimeOut timeOut, int latestVersion) { - CoreDescriptor cd = req.getCore().getCoreDescriptor(); + SolrCore core = req.getCore(); + CoreDescriptor cd = core.getCoreDescriptor(); String collection = cd.getCollectionName(); if (collection != null) { - ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader) managedIndexSchema.getResourceLoader(); if (timeOut.hasTimedOut()) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Not enough time left to update replicas. However, the schema is updated already."); } ManagedIndexSchema.waitForSchemaZkVersionAgreement(collection, cd.getCloudDescriptor().getCoreNodeName(), - latestVersion, zkLoader.getZkController(), (int) timeOut.timeLeft(TimeUnit.SECONDS)); + latestVersion, core.getCoreContainer().getZkController(), (int) timeOut.timeLeft(TimeUnit.SECONDS)); } } diff --git a/solr/core/src/java/org/apache/solr/search/SolrCoreParser.java b/solr/core/src/java/org/apache/solr/search/SolrCoreParser.java index df0127a4a68..def2919c04c 100755 --- a/solr/core/src/java/org/apache/solr/search/SolrCoreParser.java +++ b/solr/core/src/java/org/apache/solr/search/SolrCoreParser.java @@ -60,7 +60,7 @@ public class SolrCoreParser extends CoreParser implements NamedListInitializedPl if (req == null) { loader = new SolrResourceLoader(); } else { - loader = req.getSchema().getResourceLoader(); + loader = req.getCore().getResourceLoader(); } final Iterable> args = initArgs; diff --git a/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java b/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java index 9cd4c9cf63d..42f893765ea 100644 --- a/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java +++ b/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java @@ -222,9 +222,9 @@ public class SolrIndexConfig implements MapSerializable { iwc.setRAMBufferSizeMB(ramBufferSizeMB); iwc.setSimilarity(schema.getSimilarity()); - MergePolicy mergePolicy = buildMergePolicy(schema); + MergePolicy mergePolicy = buildMergePolicy(core.getResourceLoader(), schema); iwc.setMergePolicy(mergePolicy); - MergeScheduler mergeScheduler = buildMergeScheduler(schema); + MergeScheduler mergeScheduler = buildMergeScheduler(core.getResourceLoader()); iwc.setMergeScheduler(mergeScheduler); iwc.setInfoStream(infoStream); @@ -237,7 +237,7 @@ public class SolrIndexConfig implements MapSerializable { if (mergedSegmentWarmerInfo != null) { // TODO: add infostream -> normal logging system (there is an issue somewhere) - IndexReaderWarmer warmer = schema.getResourceLoader().newInstance(mergedSegmentWarmerInfo.className, + IndexReaderWarmer warmer = core.getResourceLoader().newInstance(mergedSegmentWarmerInfo.className, IndexReaderWarmer.class, null, new Class[] { InfoStream.class }, @@ -253,7 +253,7 @@ public class SolrIndexConfig implements MapSerializable { * or if no factory is configured uses the configured mergePolicy PluginInfo. */ @SuppressWarnings("unchecked") - private MergePolicy buildMergePolicy(final IndexSchema schema) { + private MergePolicy buildMergePolicy(SolrResourceLoader resourceLoader, IndexSchema schema) { final String mpfClassName; final MergePolicyFactoryArgs mpfArgs; @@ -265,20 +265,19 @@ public class SolrIndexConfig implements MapSerializable { mpfArgs = new MergePolicyFactoryArgs(mergePolicyFactoryInfo.initArgs); } - final SolrResourceLoader resourceLoader = schema.getResourceLoader(); final MergePolicyFactory mpf = resourceLoader.newInstance( mpfClassName, MergePolicyFactory.class, NO_SUB_PACKAGES, new Class[] { SolrResourceLoader.class, MergePolicyFactoryArgs.class, IndexSchema.class }, - new Object[] { resourceLoader, mpfArgs, schema }); + new Object[] {resourceLoader, mpfArgs, schema }); return mpf.getMergePolicy(); } - private MergeScheduler buildMergeScheduler(IndexSchema schema) { + private MergeScheduler buildMergeScheduler(SolrResourceLoader resourceLoader) { String msClassName = mergeSchedulerInfo == null ? SolrIndexConfig.DEFAULT_MERGE_SCHEDULER_CLASSNAME : mergeSchedulerInfo.className; - MergeScheduler scheduler = schema.getResourceLoader().newInstance(msClassName, MergeScheduler.class); + MergeScheduler scheduler = resourceLoader.newInstance(msClassName, MergeScheduler.class); if (mergeSchedulerInfo != null) { // LUCENE-5080: these two setters are removed, so we have to invoke setMaxMergesAndThreads