You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2015/11/19 18:57:25 UTC
svn commit: r1715225 - in /lucene/dev/branches/branch_5x: ./ solr/
solr/core/ solr/core/src/java/org/apache/solr/core/
solr/core/src/java/org/apache/solr/search/
solr/core/src/java/org/apache/solr/search/similarities/
solr/core/src/test-files/solr/coll...
Author: hossman
Date: Thu Nov 19 17:57:25 2015
New Revision: 1715225
URL: http://svn.apache.org/viewvc?rev=1715225&view=rev
Log:
SOLR-8280: Fixed bug in SimilarityFactory initialization that prevented SolrCoreAware factories from functioning properly with managed schema features (merge r1715215)
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/solr/ (props changed)
lucene/dev/branches/branch_5x/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/solr/core/ (props changed)
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrCore.java
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/search/ (props changed)
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/search/similarities/SchemaSimilarityFactory.java
lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-add-schema-fields-update-processor.xml
lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-rest.xml
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaSimilarityResource.java
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java
Modified: lucene/dev/branches/branch_5x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/CHANGES.txt?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Thu Nov 19 17:57:25 2015
@@ -249,6 +249,9 @@ Bug Fixes
* SOLR-8295: Fix NPE in collapse QParser when collapse field is missing from all docs in a segment (hossman)
+* SOLR-8280: Fixed bug in SimilarityFactory initialization that prevented SolrCoreAware factories -- such
+ as SchemaSimilarityFactory -- from functioning properly with managed schema features. (hossman)
+
Optimizations
----------------------
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrCore.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrCore.java?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrCore.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrCore.java Thu Nov 19 17:57:25 2015
@@ -259,17 +259,37 @@ public final class SolrCore implements S
public String getSchemaResource() {
return getLatestSchema().getResourceName();
}
-
- /** @return the latest snapshot of the schema used by this core instance. */
+
+ /**
+ * @return the latest snapshot of the schema used by this core instance.
+ * @see #setLatestSchema
+ */
public IndexSchema getLatestSchema() {
return schema;
}
-
- /** Sets the latest schema snapshot to be used by this core instance. */
+
+ /**
+ * Sets the latest schema snapshot to be used by this core instance.
+ * If the specified <code>replacementSchema</code> uses a {@link SimilarityFactory} which is
+ * {@link SolrCoreAware} then this method will {@link SolrCoreAware#inform} that factory about
+ * this SolrCore prior to using the <code>replacementSchema</code>
+ * @see #getLatestSchema
+ */
public void setLatestSchema(IndexSchema replacementSchema) {
- schema = replacementSchema;
+ // 1) For a newly instantiated core, the Similarity needs SolrCore before inform() is called on
+ // any registered SolrCoreAware listeners (which will likeley need to use the SolrIndexSearcher.
+ //
+ // 2) If a new IndexSchema is assigned to an existing live SolrCore (ie: managed schema
+ // replacement via SolrCloud) then we need to explicitly inform() the similarity because
+ // we can't rely on the normal SolrResourceLoader lifecycle because the sim was instantiated
+ // after the SolrCore was already live (see: SOLR-8311 + SOLR-8280)
+ final SimilarityFactory similarityFactory = replacementSchema.getSimilarityFactory();
+ if (similarityFactory instanceof SolrCoreAware) {
+ ((SolrCoreAware) similarityFactory).inform(this);
+ }
+ this.schema = replacementSchema;
}
-
+
public NamedList getConfigSetProperties() {
return configSetProperties;
}
@@ -751,7 +771,7 @@ public final class SolrCore implements S
this.infoRegistry = initInfoRegistry(name, config);
infoRegistry.put("fieldCache", new SolrFieldCacheMBean());
- this.schema = initSchema(config, schema);
+ initSchema(config, schema);
this.maxWarmingSearchers = config.maxWarmingSearchers;
this.slowQueryThresholdMillis = config.slowQueryThresholdMillis;
@@ -948,17 +968,18 @@ public final class SolrCore implements S
infoRegistry.put("updateHandler", newUpdateHandler);
return newUpdateHandler;
}
-
- private IndexSchema initSchema(SolrConfig config, IndexSchema schema) {
+
+ /**
+ * Initializes the "Latest Schema" for this SolrCore using either the provided <code>schema</code>
+ * if non-null, or a new instance build via the factory identified in the specified <code>config</code>
+ * @see IndexSchemaFactory
+ * @see #setLatestSchema
+ */
+ private void initSchema(SolrConfig config, IndexSchema schema) {
if (schema == null) {
schema = IndexSchemaFactory.buildIndexSchema(IndexSchema.DEFAULT_SCHEMA_FILE, config);
}
- final SimilarityFactory similarityFactory = schema.getSimilarityFactory();
- if (similarityFactory instanceof SolrCoreAware) {
- // Similarity needs SolrCore before inform() is called on all registered SolrCoreAware listeners below
- ((SolrCoreAware) similarityFactory).inform(this);
- }
- return schema;
+ setLatestSchema(schema);
}
private Map<String,SolrInfoMBean> initInfoRegistry(String name, SolrConfig config) {
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java Thu Nov 19 17:57:25 2015
@@ -745,6 +745,8 @@ public class SolrResourceLoader implemen
awareCompatibility = new HashMap<>();
awareCompatibility.put(
SolrCoreAware.class, new Class[] {
+ // DO NOT ADD THINGS TO THIS LIST -- ESPECIALLY THINGS THAT CAN BE CREATED DYNAMICALLY
+ // VIA RUNTIME APIS -- UNTILL CAREFULLY CONSIDERING THE ISSUES MENTIONED IN SOLR-8311
CodecFactory.class,
DirectoryFactory.class,
ManagedIndexSchemaFactory.class,
@@ -759,6 +761,8 @@ public class SolrResourceLoader implemen
awareCompatibility.put(
ResourceLoaderAware.class, new Class[] {
+ // DO NOT ADD THINGS TO THIS LIST -- ESPECIALLY THINGS THAT CAN BE CREATED DYNAMICALLY
+ // VIA RUNTIME APIS -- UNTILL CAREFULLY CONSIDERING THE ISSUES MENTIONED IN SOLR-8311
CharFilterFactory.class,
TokenFilterFactory.class,
TokenizerFactory.class,
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/search/similarities/SchemaSimilarityFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/search/similarities/SchemaSimilarityFactory.java?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/search/similarities/SchemaSimilarityFactory.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/search/similarities/SchemaSimilarityFactory.java Thu Nov 19 17:57:25 2015
@@ -26,6 +26,9 @@ import org.apache.solr.schema.FieldType;
import org.apache.solr.schema.SimilarityFactory;
import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
/**
* SimilarityFactory that returns a {@link PerFieldSimilarityWrapper}
* that delegates to the field type, if it's configured, otherwise
@@ -44,8 +47,9 @@ import org.apache.solr.util.plugin.SolrC
* @see FieldType#getSimilarity
*/
public class SchemaSimilarityFactory extends SimilarityFactory implements SolrCoreAware {
- private Similarity similarity;
+ private Similarity similarity; // set by init
private Similarity defaultSimilarity = new ClassicSimilarity();
+
private volatile SolrCore core;
@Override
@@ -72,7 +76,9 @@ public class SchemaSimilarityFactory ext
@Override
public Similarity getSimilarity() {
- assert core != null : "inform must be called first";
+ if (null == core) {
+ throw new IllegalStateException("SchemaSimilarityFactory can not be used until SolrCoreAware.inform has been called");
+ }
return similarity;
}
}
Modified: lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-add-schema-fields-update-processor.xml
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-add-schema-fields-update-processor.xml?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-add-schema-fields-update-processor.xml (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-add-schema-fields-update-processor.xml Thu Nov 19 17:57:25 2015
@@ -47,4 +47,9 @@
</fields>
<uniqueKey>id</uniqueKey>
+
+ <!-- testing with a similarity that is SolrCoreAware -->
+ <similarity class="solr.SchemaSimilarityFactory"/>
+
+
</schema>
Modified: lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-rest.xml
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-rest.xml?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-rest.xml (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test-files/solr/collection1/conf/schema-rest.xml Thu Nov 19 17:57:25 2015
@@ -634,4 +634,5 @@
<solrQueryParser defaultOperator="OR"/>
+ <similarity class="solr.SchemaSimilarityFactory" />
</schema>
Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java Thu Nov 19 17:57:25 2015
@@ -18,8 +18,19 @@ package org.apache.solr.rest.schema;
*/
import org.apache.commons.io.FileUtils;
+
+import org.apache.lucene.search.similarities.PerFieldSimilarityWrapper;
+import org.apache.lucene.search.similarities.ClassicSimilarity;
+import org.apache.lucene.misc.SweetSpotSimilarity;
+import org.apache.lucene.search.similarities.Similarity;
+
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.schema.SimilarityFactory;
+import org.apache.solr.search.similarities.SchemaSimilarityFactory;
import org.apache.solr.util.RestTestBase;
import org.apache.solr.util.RestTestHarness;
+
import org.junit.After;
import org.junit.Before;
import org.noggit.JSONParser;
@@ -356,18 +367,21 @@ public class TestBulkSchemaAPI extends R
assertNotNull(m);
m = (Map)m.get("similarity");
assertNotNull(m);
- assertEquals("org.apache.lucene.misc.SweetSpotSimilarity", m.get("class"));
+ assertEquals(SweetSpotSimilarity.class.getName(), m.get("class"));
m = getObj(harness, "a4", "fields");
assertNotNull("field a4 not created", m);
assertEquals("mySimField", m.get("type"));
-
+ assertFieldSimilarity("a4", SweetSpotSimilarity.class);
+
m = getObj(harness, "myWhitespaceTxtField", "fieldTypes");
assertNotNull(m);
+ assertNull(m.get("similarity")); // unspecified, expect default
m = getObj(harness, "a5", "fields");
assertNotNull("field a5 not created", m);
assertEquals("myWhitespaceTxtField", m.get("type"));
+ assertFieldSimilarity("a5", ClassicSimilarity.class); // unspecified, expect default
m = getObj(harness, "wdf_nocase", "fields");
assertNull("field 'wdf_nocase' not deleted", m);
@@ -665,4 +679,29 @@ public class TestBulkSchemaAPI extends R
}
return result;
}
+
+ /**
+ * whitebox checks the Similarity for the specified field according to {@link SolrCore#getLatestSchema}
+ */
+ private static void assertFieldSimilarity(String fieldname, Class<? extends Similarity> expected) {
+ CoreContainer cc = jetty.getCoreContainer();
+ try (SolrCore core = cc.getCore("collection1")) {
+ SimilarityFactory simfac = core.getLatestSchema().getSimilarityFactory();
+ assertNotNull(simfac);
+ assertTrue("test only works with SchemaSimilarityFactory",
+ simfac instanceof SchemaSimilarityFactory);
+
+ Similarity mainSim = core.getLatestSchema().getSimilarity();
+ assertNotNull(mainSim);
+
+ // sanity check simfac vs sim in use - also verify infom called on simfac, otherwise exception
+ assertEquals(mainSim, simfac.getSimilarity());
+
+ assertTrue("test only works with PerFieldSimilarityWrapper, SchemaSimilarityFactory redefined?",
+ mainSim instanceof PerFieldSimilarityWrapper);
+ Similarity fieldSim = ((PerFieldSimilarityWrapper)mainSim).get(fieldname);
+ assertEquals("wrong sim for field=" + fieldname, expected, fieldSim.getClass());
+
+ }
+ }
}
Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaSimilarityResource.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaSimilarityResource.java?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaSimilarityResource.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/rest/schema/TestSchemaSimilarityResource.java Thu Nov 19 17:57:25 2015
@@ -24,7 +24,7 @@ public class TestSchemaSimilarityResourc
public void testGetSchemaSimilarity() throws Exception {
assertQ("/schema/similarity?indent=on&wt=xml",
"count(/response/lst[@name='similarity']) = 1",
- "/response/lst[@name='similarity']/str[@name='class'][.='org.apache.solr.search.similarities.ClassicSimilarityFactory']");
+ "/response/lst[@name='similarity']/str[@name='class'][.='solr.SchemaSimilarityFactory']");
}
}
Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java?rev=1715225&r1=1715224&r2=1715225&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java Thu Nov 19 17:57:25 2015
@@ -22,21 +22,52 @@ import java.io.IOException;
import org.apache.commons.codec.Charsets;
import org.apache.commons.io.FileUtils;
+
+import org.apache.lucene.search.similarities.Similarity;
+
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrCore;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.schema.SimilarityFactory;
+import org.apache.solr.search.similarities.LMJelinekMercerSimilarityFactory;
+import org.apache.solr.search.similarities.SchemaSimilarityFactory;
import org.apache.solr.update.AddUpdateCommand;
import org.apache.solr.update.CommitUpdateCommand;
import org.apache.solr.update.UpdateHandler;
+import org.apache.solr.util.plugin.SolrCoreAware;
import org.junit.BeforeClass;
import org.junit.Test;
public class ChangedSchemaMergeTest extends SolrTestCaseJ4 {
+
+ public static Class<? extends SimilarityFactory> simfac1;
+ public static Class<? extends SimilarityFactory> simfac2;
+
@BeforeClass
public static void beforeClass() throws Exception {
+
+ simfac1 = LMJelinekMercerSimilarityFactory.class;
+ simfac2 = SchemaSimilarityFactory.class;
+
+ // sanity check our test...
+ assertTrue("Effectiveness of tets depends on SchemaSimilarityFactory being SolrCoreAware " +
+ "something changed in the impl and now major portions of this test are useless",
+ SolrCoreAware.class.isAssignableFrom(simfac2));
+
+ // randomize the order these similarities are used in the changed schemas
+ // to help test proper initialization in both code paths
+ if (random().nextBoolean()) {
+ Class<? extends SimilarityFactory> tmp = simfac1;
+ simfac1 = simfac2;
+ simfac2 = tmp;
+ }
+ System.setProperty("solr.test.simfac1", simfac1.getName());
+ System.setProperty("solr.test.simfac2", simfac2.getName());
+
initCore();
}
@@ -66,12 +97,29 @@ public class ChangedSchemaMergeTest exte
return cores;
}
+ public void testSanityOfSchemaSimilarityFactoryInform() {
+ // sanity check that SchemaSimilarityFactory will throw an Exception if you
+ // try to use it w/o inform(SolrCoreAware) otherwise assertSimilarity is useless
+ SchemaSimilarityFactory broken = new SchemaSimilarityFactory();
+ broken.init(new ModifiableSolrParams());
+ // NO INFORM
+ try {
+ Similarity bogus = broken.getSimilarity();
+ fail("SchemaSimilarityFactory should have thrown IllegalStateException b/c inform not used");
+ } catch (IllegalStateException expected) {
+ assertTrue("GOT: " + expected.getMessage(),
+ expected.getMessage().contains("SolrCoreAware.inform"));
+ }
+ }
+
@Test
public void testOptimizeDiffSchemas() throws Exception {
// load up a core (why not put it on disk?)
CoreContainer cc = init();
try (SolrCore changed = cc.getCore("changed")) {
+ assertSimilarity(changed, simfac1);
+
// add some documents
addDoc(changed, "id", "1", "which", "15", "text", "some stuff with which");
addDoc(changed, "id", "2", "which", "15", "text", "some stuff with which");
@@ -85,6 +133,10 @@ public class ChangedSchemaMergeTest exte
IndexSchema iSchema = IndexSchemaFactory.buildIndexSchema("schema.xml", changed.getSolrConfig());
changed.setLatestSchema(iSchema);
+
+ assertSimilarity(changed, simfac2);
+ // sanity check our sanity check
+ assertFalse("test is broken: both simfacs are the same", simfac1.equals(simfac2));
addDoc(changed, "id", "1", "text", "some stuff without which");
addDoc(changed, "id", "5", "text", "some stuff without which");
@@ -96,6 +148,13 @@ public class ChangedSchemaMergeTest exte
}
}
+ private static void assertSimilarity(SolrCore core, Class<? extends SimilarityFactory> expected) {
+ SimilarityFactory actual = core.getLatestSchema().getSimilarityFactory();
+ assertNotNull(actual);
+ assertEquals(expected, actual.getClass());
+ // if SolrCoreAware sim isn't properly initialized, this will throw an exception
+ assertNotNull(actual.getSimilarity());
+ }
private static String withWhich = "<schema name=\"tiny\" version=\"1.1\">\n" +
" <fields>\n" +
@@ -116,6 +175,7 @@ public class ChangedSchemaMergeTest exte
" <fieldType name=\"string\" class=\"solr.StrField\"/>\n" +
" <fieldType name=\"int\" class=\"solr.TrieIntField\" precisionStep=\"0\" positionIncrementGap=\"0\"/>" +
" </types>\n" +
+ " <similarity class=\"${solr.test.simfac1}\"/> " +
"</schema>";
private static String withoutWhich = "<schema name=\"tiny\" version=\"1.1\">\n" +
@@ -135,6 +195,7 @@ public class ChangedSchemaMergeTest exte
" <fieldType name=\"string\" class=\"solr.StrField\"/>\n" +
" <fieldType name=\"int\" class=\"solr.TrieIntField\" precisionStep=\"0\" positionIncrementGap=\"0\"/>" +
" </types>\n" +
+ " <similarity class=\"${solr.test.simfac2}\"/> " +
"</schema>";