You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2019/03/19 17:52:46 UTC
[lucene-solr] branch branch_8x updated: SOLR-13253: avoid using
IndexSchema.getResourceLoader for non-schema things. Furthermore it's
reference to SolrConfig was removed.
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 8f25ded SOLR-13253: avoid using IndexSchema.getResourceLoader for non-schema things. Furthermore it's reference to SolrConfig was removed.
8f25ded is described below
commit 8f25ded16f190de22f0d0816e12cf2a40eb19a83
Author: David Smiley <ds...@apache.org>
AuthorDate: Tue Mar 19 13:51:44 2019 -0400
SOLR-13253: avoid using IndexSchema.getResourceLoader for non-schema things.
Furthermore it's reference to SolrConfig was removed.
(cherry picked from commit 85a702cdff23a6352945dd78eb54ff6db68f6965)
---
solr/CHANGES.txt | 3 ++
.../src/java/org/apache/solr/core/SolrCore.java | 2 +-
.../java/org/apache/solr/schema/IndexSchema.java | 37 ++++++++++++----------
.../org/apache/solr/schema/IndexSchemaFactory.java | 12 +++----
.../org/apache/solr/schema/ManagedIndexSchema.java | 30 +++++-------------
.../solr/schema/ManagedIndexSchemaFactory.java | 15 ++-------
.../java/org/apache/solr/schema/SchemaManager.java | 6 ++--
.../org/apache/solr/search/SolrCoreParser.java | 2 +-
.../org/apache/solr/update/SolrIndexConfig.java | 15 ++++-----
9 files changed, 51 insertions(+), 71 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 5f61fc9..e047365 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -105,6 +105,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 93bd690..c86ceb4 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 ce623e6..8042e80 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<SchemaField, Integer> 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 <em>not</em> 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 fda4868..4234ba1 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 0314ad9..c7fbf27 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 e433dc4..72c3d6f 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 217c726..afc3b04 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 df0127a..def2919 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<Map.Entry<String,Object>> 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 9cd4c9c..42f8937 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