Apache
/
lucene
mirror of https://github.com/apache/lucene.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SOLR-8280: Fixed bug in SimilarityFactory initialization that prevented SolrCoreAware factories from functioning properly with managed schema features 

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1715215 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Chris M. Hostetter 2015-11-19 17:03:36 +00:00
parent 491bb5ba90
commit 766b017b1a
9 changed files with 159 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
solr/CHANGES.txt
View File

@ -393,6 +393,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
----------------------

51
solr/core/src/java/org/apache/solr/core/SolrCore.java
View File

@ -259,17 +259,37 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
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 SolrInfoMBean, Closeable {
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 SolrInfoMBean, Closeable {
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) {

4
solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
View File

@ -745,6 +745,8 @@ public class SolrResourceLoader implements ResourceLoader,Closeable
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 implements ResourceLoader,Closeable
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,

8
solr/core/src/java/org/apache/solr/search/similarities/SchemaSimilarityFactory.java
View File

@ -29,6 +29,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;
/**
* <p>
* SimilarityFactory that returns a {@link PerFieldSimilarityWrapper}
@ -52,6 +55,7 @@ import org.apache.solr.util.plugin.SolrCoreAware;
* @see FieldType#getSimilarity
*/
public class SchemaSimilarityFactory extends SimilarityFactory implements SolrCoreAware {
private Similarity similarity; // set by init
private Similarity defaultSimilarity; // set by inform(SolrCore)
private volatile SolrCore core;
@ -83,7 +87,9 @@ public class SchemaSimilarityFactory extends SimilarityFactory implements SolrCo
@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;
}
}

5
solr/core/src/test-files/solr/collection1/conf/schema-add-schema-fields-update-processor.xml
View File

@ -47,4 +47,9 @@
</fields>
<uniqueKey>id</uniqueKey>
<!-- testing with a similarity that is SolrCoreAware -->
<similarity class="solr.SchemaSimilarityFactory"/>
</schema>

1
solr/core/src/test-files/solr/collection1/conf/schema-rest.xml
View File

@ -634,4 +634,5 @@
<solrQueryParser defaultOperator="OR"/>
<similarity class="solr.SchemaSimilarityFactory" />
</schema>

43
solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
View File

@ -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.BM25Similarity;
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 RestTestBase {
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", BM25Similarity.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 RestTestBase {
}
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());
}
}
}

2
solr/core/src/test/org/apache/solr/rest/schema/TestSchemaSimilarityResource.java
View File

@ -24,7 +24,7 @@ public class TestSchemaSimilarityResource extends SolrRestletTestBase {
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.BM25SimilarityFactory']");
"/response/lst[@name='similarity']/str[@name='class'][.='solr.SchemaSimilarityFactory']");
}
}

61
solr/core/src/test/org/apache/solr/schema/ChangedSchemaMergeTest.java
View File

@ -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 extends SolrTestCaseJ4 {
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 extends SolrTestCaseJ4 {
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");
@ -99,6 +151,13 @@ public class ChangedSchemaMergeTest extends SolrTestCaseJ4 {
}
}
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" +
@ -119,6 +178,7 @@ public class ChangedSchemaMergeTest extends SolrTestCaseJ4 {
" <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" +
@ -138,6 +198,7 @@ public class ChangedSchemaMergeTest extends SolrTestCaseJ4 {
" <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>";