SOLR-13253: avoid using IndexSchema.getResourceLoader for non-schema things.

Furthermore it's reference to SolrConfig was removed.
This commit is contained in:
David Smiley 2019-03-19 13:51:44 -04:00
parent 5b7866b085
commit 85a702cdff
9 changed files with 51 additions and 71 deletions

View File

@ -126,6 +126,9 @@ Bug Fixes
* SOLR-13244: Admin UI Nodes view fails and is empty when a node is temporarily down (janhoy) * 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 Improvements
---------------------- ----------------------

View File

@ -1388,7 +1388,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
final PluginInfo info = solrConfig.getPluginInfo(CodecFactory.class.getName()); final PluginInfo info = solrConfig.getPluginInfo(CodecFactory.class.getName());
final CodecFactory factory; final CodecFactory factory;
if (info != null) { if (info != null) {
factory = schema.getResourceLoader().newInstance(info.className, CodecFactory.class); factory = resourceLoader.newInstance(info.className, CodecFactory.class);
factory.init(info.initArgs); factory.init(info.initArgs);
} else { } else {
factory = new CodecFactory() { factory = new CodecFactory() {

View File

@ -33,6 +33,7 @@ import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map; import java.util.Map;
import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.SortedMap; import java.util.SortedMap;
import java.util.TreeMap; 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.NamedList;
import org.apache.solr.common.util.Pair; import org.apache.solr.common.util.Pair;
import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.SimpleOrderedMap;
import org.apache.solr.core.XmlConfigFile; import org.apache.solr.core.SolrCore;
import org.apache.solr.core.SolrConfig;
import org.apache.solr.core.SolrResourceLoader; import org.apache.solr.core.SolrResourceLoader;
import org.apache.solr.core.XmlConfigFile;
import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.response.SchemaXmlWriter; import org.apache.solr.response.SchemaXmlWriter;
import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.response.SolrQueryResponse;
@ -126,9 +127,9 @@ public class IndexSchema {
private static final String XPATH_OR = " | "; private static final String XPATH_OR = " | ";
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
protected final SolrConfig solrConfig;
protected String resourceName; protected String resourceName;
protected String name; protected String name;
protected final Version luceneVersion;
protected float version; protected float version;
protected final SolrResourceLoader loader; protected final SolrResourceLoader loader;
@ -161,20 +162,16 @@ public class IndexSchema {
*/ */
protected Map<SchemaField, Integer> copyFieldTargetCounts = new HashMap<>(); protected Map<SchemaField, Integer> copyFieldTargetCounts = new HashMap<>();
/** /**
* Constructs a schema using the specified resource name and stream. * Constructs a schema using the specified resource name and stream.
* @see SolrResourceLoader#openSchema * @see SolrResourceLoader#openSchema
* By default, this follows the normal config path directory searching rules. * By default, this follows the normal config path directory searching rules.
* @see SolrResourceLoader#openResource * @see SolrResourceLoader#openResource
*/ */
public IndexSchema(SolrConfig solrConfig, String name, InputSource is) { public IndexSchema(String name, InputSource is, Version luceneVersion, SolrResourceLoader resourceLoader) {
assert null != solrConfig : "SolrConfig should never be null"; this(luceneVersion, resourceLoader);
assert null != name : "schema resource name should never be null";
assert null != is : "schema InputSource should never be null";
this.solrConfig = solrConfig; this.resourceName = Objects.requireNonNull(name);
this.resourceName = name;
loader = solrConfig.getResourceLoader();
try { try {
readSchema(is); readSchema(is);
loader.inform(loader); 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 <em>not</em> be used for any other purpose or time;
* consider {@link SolrCore#getResourceLoader()} instead.
* @since solr 1.4 * @since solr 1.4
*/ */
public SolrResourceLoader getResourceLoader() { public SolrResourceLoader getResourceLoader() {
//TODO consider asserting the schema has not finished loading somehow?
return loader; return loader;
} }
@ -207,7 +214,7 @@ public class IndexSchema {
/** The Default Lucene Match Version for this IndexSchema */ /** The Default Lucene Match Version for this IndexSchema */
public Version getDefaultLuceneMatchVersion() { public Version getDefaultLuceneMatchVersion() {
return solrConfig.luceneMatchVersion; return luceneVersion;
} }
public float getVersion() { public float getVersion() {
@ -443,6 +450,7 @@ public class IndexSchema {
} }
protected void readSchema(InputSource is) { protected void readSchema(InputSource is) {
assert null != is : "schema InputSource should never be null";
try { try {
// pass the config resource loader to avoid building an empty one for no reason: // 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 // 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; 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 * Copies this schema, adds the given field to the copy
* Requires synchronizing on the object returned by * Requires synchronizing on the object returned by

View File

@ -16,10 +16,14 @@
*/ */
package org.apache.solr.schema; 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;
import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.core.PluginInfo; 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.core.SolrResourceLoader;
import org.apache.solr.util.SystemIdResolver; import org.apache.solr.util.SystemIdResolver;
import org.apache.solr.util.plugin.NamedListInitializedPlugin; import org.apache.solr.util.plugin.NamedListInitializedPlugin;
@ -27,10 +31,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.xml.sax.InputSource; 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 */ /** Base class for factories for IndexSchema implementations */
public abstract class IndexSchemaFactory implements NamedListInitializedPlugin { public abstract class IndexSchemaFactory implements NamedListInitializedPlugin {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); 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 inputSource = new InputSource(schemaInputStream);
inputSource.setSystemId(SystemIdResolver.createSystemIdFromResourceName(resourceName)); inputSource.setSystemId(SystemIdResolver.createSystemIdFromResourceName(resourceName));
IndexSchema schema = new IndexSchema(config, resourceName, inputSource); IndexSchema schema = new IndexSchema(resourceName, inputSource, config.luceneMatchVersion, loader);
return schema; return schema;
} }

View File

@ -44,6 +44,7 @@ import org.apache.lucene.analysis.util.CharFilterFactory;
import org.apache.lucene.analysis.util.ResourceLoaderAware; import org.apache.lucene.analysis.util.ResourceLoaderAware;
import org.apache.lucene.analysis.util.TokenFilterFactory; import org.apache.lucene.analysis.util.TokenFilterFactory;
import org.apache.lucene.analysis.util.TokenizerFactory; import org.apache.lucene.analysis.util.TokenizerFactory;
import org.apache.lucene.util.Version;
import org.apache.solr.analysis.TokenizerChain; import org.apache.solr.analysis.TokenizerChain;
import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrRequest;
@ -100,9 +101,8 @@ public final class ManagedIndexSchema extends IndexSchema {
* @see org.apache.solr.core.SolrResourceLoader#openResource * @see org.apache.solr.core.SolrResourceLoader#openResource
*/ */
ManagedIndexSchema(SolrConfig solrConfig, String name, InputSource is, boolean isMutable, ManagedIndexSchema(SolrConfig solrConfig, String name, InputSource is, boolean isMutable,
String managedSchemaResourceName, int schemaZkVersion, Object schemaUpdateLock) String managedSchemaResourceName, int schemaZkVersion, Object schemaUpdateLock) {
throws KeeperException, InterruptedException { super(name, is, solrConfig.luceneMatchVersion, solrConfig.getResourceLoader());
super(solrConfig, name, is);
this.isMutable = isMutable; this.isMutable = isMutable;
this.managedSchemaResourceName = managedSchemaResourceName; this.managedSchemaResourceName = managedSchemaResourceName;
this.schemaZkVersion = schemaZkVersion; this.schemaZkVersion = schemaZkVersion;
@ -1320,10 +1320,9 @@ public final class ManagedIndexSchema extends IndexSchema {
} }
} }
private ManagedIndexSchema(final SolrConfig solrConfig, final SolrResourceLoader loader, boolean isMutable, private ManagedIndexSchema(Version luceneVersion, SolrResourceLoader loader, boolean isMutable,
String managedSchemaResourceName, int schemaZkVersion, Object schemaUpdateLock) String managedSchemaResourceName, int schemaZkVersion, Object schemaUpdateLock) {
throws KeeperException, InterruptedException { super(luceneVersion, loader);
super(solrConfig, loader);
this.isMutable = isMutable; this.isMutable = isMutable;
this.managedSchemaResourceName = managedSchemaResourceName; this.managedSchemaResourceName = managedSchemaResourceName;
this.schemaZkVersion = schemaZkVersion; this.schemaZkVersion = schemaZkVersion;
@ -1340,22 +1339,9 @@ public final class ManagedIndexSchema extends IndexSchema {
* @return A shallow copy of this schema * @return A shallow copy of this schema
*/ */
ManagedIndexSchema shallowCopy(boolean includeFieldDataStructures) { ManagedIndexSchema shallowCopy(boolean includeFieldDataStructures) {
ManagedIndexSchema newSchema = null; ManagedIndexSchema newSchema = new ManagedIndexSchema
try { (luceneVersion, loader, isMutable, managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock());
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);
}
assert newSchema != null;
newSchema.name = name; newSchema.name = name;
newSchema.version = version; newSchema.version = version;
newSchema.similarity = similarity; newSchema.similarity = similarity;

View File

@ -168,19 +168,8 @@ public class ManagedIndexSchemaFactory extends IndexSchemaFactory implements Sol
} }
InputSource inputSource = new InputSource(schemaInputStream); InputSource inputSource = new InputSource(schemaInputStream);
inputSource.setSystemId(SystemIdResolver.createSystemIdFromResourceName(loadedResource)); inputSource.setSystemId(SystemIdResolver.createSystemIdFromResourceName(loadedResource));
try { schema = new ManagedIndexSchema(config, loadedResource, inputSource, isMutable,
schema = new ManagedIndexSchema(config, loadedResource, inputSource, isMutable, managedSchemaResourceName, schemaZkVersion, getSchemaUpdateLock());
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);
}
if (shouldUpgrade) { if (shouldUpgrade) {
// Persist the managed schema if it doesn't already exist // Persist the managed schema if it doesn't already exist
upgradeToManagedSchema(); upgradeToManagedSchema();

View File

@ -159,16 +159,16 @@ public class SchemaManager {
} }
private void waitForOtherReplicasToUpdate(TimeOut timeOut, int latestVersion) { private void waitForOtherReplicasToUpdate(TimeOut timeOut, int latestVersion) {
CoreDescriptor cd = req.getCore().getCoreDescriptor(); SolrCore core = req.getCore();
CoreDescriptor cd = core.getCoreDescriptor();
String collection = cd.getCollectionName(); String collection = cd.getCollectionName();
if (collection != null) { if (collection != null) {
ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader) managedIndexSchema.getResourceLoader();
if (timeOut.hasTimedOut()) { if (timeOut.hasTimedOut()) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
"Not enough time left to update replicas. However, the schema is updated already."); "Not enough time left to update replicas. However, the schema is updated already.");
} }
ManagedIndexSchema.waitForSchemaZkVersionAgreement(collection, cd.getCloudDescriptor().getCoreNodeName(), ManagedIndexSchema.waitForSchemaZkVersionAgreement(collection, cd.getCloudDescriptor().getCoreNodeName(),
latestVersion, zkLoader.getZkController(), (int) timeOut.timeLeft(TimeUnit.SECONDS)); latestVersion, core.getCoreContainer().getZkController(), (int) timeOut.timeLeft(TimeUnit.SECONDS));
} }
} }

View File

@ -60,7 +60,7 @@ public class SolrCoreParser extends CoreParser implements NamedListInitializedPl
if (req == null) { if (req == null) {
loader = new SolrResourceLoader(); loader = new SolrResourceLoader();
} else { } else {
loader = req.getSchema().getResourceLoader(); loader = req.getCore().getResourceLoader();
} }
final Iterable<Map.Entry<String,Object>> args = initArgs; final Iterable<Map.Entry<String,Object>> args = initArgs;

View File

@ -222,9 +222,9 @@ public class SolrIndexConfig implements MapSerializable {
iwc.setRAMBufferSizeMB(ramBufferSizeMB); iwc.setRAMBufferSizeMB(ramBufferSizeMB);
iwc.setSimilarity(schema.getSimilarity()); iwc.setSimilarity(schema.getSimilarity());
MergePolicy mergePolicy = buildMergePolicy(schema); MergePolicy mergePolicy = buildMergePolicy(core.getResourceLoader(), schema);
iwc.setMergePolicy(mergePolicy); iwc.setMergePolicy(mergePolicy);
MergeScheduler mergeScheduler = buildMergeScheduler(schema); MergeScheduler mergeScheduler = buildMergeScheduler(core.getResourceLoader());
iwc.setMergeScheduler(mergeScheduler); iwc.setMergeScheduler(mergeScheduler);
iwc.setInfoStream(infoStream); iwc.setInfoStream(infoStream);
@ -237,7 +237,7 @@ public class SolrIndexConfig implements MapSerializable {
if (mergedSegmentWarmerInfo != null) { if (mergedSegmentWarmerInfo != null) {
// TODO: add infostream -> normal logging system (there is an issue somewhere) // 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, IndexReaderWarmer.class,
null, null,
new Class[] { InfoStream.class }, new Class[] { InfoStream.class },
@ -253,7 +253,7 @@ public class SolrIndexConfig implements MapSerializable {
* or if no factory is configured uses the configured mergePolicy PluginInfo. * or if no factory is configured uses the configured mergePolicy PluginInfo.
*/ */
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
private MergePolicy buildMergePolicy(final IndexSchema schema) { private MergePolicy buildMergePolicy(SolrResourceLoader resourceLoader, IndexSchema schema) {
final String mpfClassName; final String mpfClassName;
final MergePolicyFactoryArgs mpfArgs; final MergePolicyFactoryArgs mpfArgs;
@ -265,20 +265,19 @@ public class SolrIndexConfig implements MapSerializable {
mpfArgs = new MergePolicyFactoryArgs(mergePolicyFactoryInfo.initArgs); mpfArgs = new MergePolicyFactoryArgs(mergePolicyFactoryInfo.initArgs);
} }
final SolrResourceLoader resourceLoader = schema.getResourceLoader();
final MergePolicyFactory mpf = resourceLoader.newInstance( final MergePolicyFactory mpf = resourceLoader.newInstance(
mpfClassName, mpfClassName,
MergePolicyFactory.class, MergePolicyFactory.class,
NO_SUB_PACKAGES, NO_SUB_PACKAGES,
new Class[] { SolrResourceLoader.class, MergePolicyFactoryArgs.class, IndexSchema.class }, new Class[] { SolrResourceLoader.class, MergePolicyFactoryArgs.class, IndexSchema.class },
new Object[] { resourceLoader, mpfArgs, schema }); new Object[] {resourceLoader, mpfArgs, schema });
return mpf.getMergePolicy(); return mpf.getMergePolicy();
} }
private MergeScheduler buildMergeScheduler(IndexSchema schema) { private MergeScheduler buildMergeScheduler(SolrResourceLoader resourceLoader) {
String msClassName = mergeSchedulerInfo == null ? SolrIndexConfig.DEFAULT_MERGE_SCHEDULER_CLASSNAME : mergeSchedulerInfo.className; 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) { if (mergeSchedulerInfo != null) {
// LUCENE-5080: these two setters are removed, so we have to invoke setMaxMergesAndThreads // LUCENE-5080: these two setters are removed, so we have to invoke setMaxMergesAndThreads