Browse Source

Remove classic similarity (#46078)

This commit removes the `classic` similarity from code and docs in master (8.0). The `classic` similarity cannot be used on indices created after 7.0.

Closes #46058
Jim Ferenczi 6 years ago
parent
commit
55d4581ee4

+ 1 - 6
docs/reference/mapping/params/similarity.asciidoc

@@ -3,7 +3,7 @@
 
 Elasticsearch allows you to configure a scoring algorithm or _similarity_ per
 field. The `similarity` setting provides a simple way of choosing a similarity
-algorithm other than the default `BM25`, such as `TF/IDF`.
+algorithm other than the default `BM25`, such as `boolean`.
 
 Similarities are mostly useful for <<text,`text`>> fields, but can also apply
 to other field types.
@@ -20,11 +20,6 @@ configuration are:
         See {defguide}/pluggable-similarites.html[Pluggable Similarity Algorithms]
         for more information.
 
-`classic`::
-        The TF/IDF algorithm which used to be the default in Elasticsearch and
-        Lucene. See {defguide}/practical-scoring-function.html[Lucene’s Practical Scoring Function]
-        for more information.
-
 `boolean`::
         A simple boolean similarity, which is used when full-text ranking is not needed
         and the score should only be based on whether the query terms match or not.

+ 0 - 11
server/src/main/java/org/elasticsearch/index/similarity/SimilarityProviders.java

@@ -29,7 +29,6 @@ import org.apache.lucene.search.similarities.BasicModelIF;
 import org.apache.lucene.search.similarities.BasicModelIn;
 import org.apache.lucene.search.similarities.BasicModelIne;
 import org.apache.lucene.search.similarities.BooleanSimilarity;
-import org.apache.lucene.search.similarities.ClassicSimilarity;
 import org.apache.lucene.search.similarities.DFISimilarity;
 import org.apache.lucene.search.similarities.DFRSimilarity;
 import org.apache.lucene.search.similarities.Distribution;
@@ -259,16 +258,6 @@ final class SimilarityProviders {
         return new BooleanSimilarity();
     }
 
-    public static ClassicSimilarity createClassicSimilarity(Settings settings, Version indexCreatedVersion) {
-        assertSettingsIsSubsetOf("classic", indexCreatedVersion, settings, DISCOUNT_OVERLAPS);
-
-        boolean discountOverlaps = settings.getAsBoolean(DISCOUNT_OVERLAPS, true);
-
-        ClassicSimilarity similarity = new ClassicSimilarity();
-        similarity.setDiscountOverlaps(discountOverlaps);
-        return similarity;
-    }
-
     public static DFRSimilarity createDfrSimilarity(Settings settings, Version indexCreatedVersion) {
         assertSettingsIsSubsetOf("DFR", indexCreatedVersion, settings,
                 "basic_model", "after_effect", "normalization",

+ 0 - 30
server/src/main/java/org/elasticsearch/index/similarity/SimilarityService.java

@@ -26,7 +26,6 @@ import org.apache.lucene.search.CollectionStatistics;
 import org.apache.lucene.search.Explanation;
 import org.apache.lucene.search.TermStatistics;
 import org.apache.lucene.search.similarities.BooleanSimilarity;
-import org.apache.lucene.search.similarities.ClassicSimilarity;
 import org.apache.lucene.search.similarities.PerFieldSimilarityWrapper;
 import org.apache.lucene.search.similarities.Similarity;
 import org.apache.lucene.search.similarities.Similarity.SimScorer;
@@ -53,27 +52,10 @@ public final class SimilarityService extends AbstractIndexComponent {
 
     private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(SimilarityService.class));
     public static final String DEFAULT_SIMILARITY = "BM25";
-    private static final String CLASSIC_SIMILARITY = "classic";
     private static final Map<String, Function<Version, Supplier<Similarity>>> DEFAULTS;
     public static final Map<String, TriFunction<Settings, Version, ScriptService, Similarity>> BUILT_IN;
     static {
         Map<String, Function<Version, Supplier<Similarity>>> defaults = new HashMap<>();
-        defaults.put(CLASSIC_SIMILARITY, version -> {
-            if (version.onOrAfter(Version.V_7_0_0)) {
-                return () -> {
-                    throw new IllegalArgumentException("The [classic] similarity may not be used anymore. Please use the [BM25] "
-                            + "similarity or build a custom [scripted] similarity instead.");
-                };
-            } else {
-                final ClassicSimilarity similarity = SimilarityProviders.createClassicSimilarity(Settings.EMPTY, version);
-                return () -> {
-                    deprecationLogger.deprecated("The [classic] similarity is now deprecated in favour of BM25, which is generally "
-                            + "accepted as a better alternative. Use the [BM25] similarity or build a custom [scripted] similarity "
-                            + "instead.");
-                    return similarity;
-                };
-            }
-        });
         defaults.put("BM25", version -> {
             final LegacyBM25Similarity similarity = SimilarityProviders.createBM25Similarity(Settings.EMPTY, version);
             return () -> similarity;
@@ -84,18 +66,6 @@ public final class SimilarityService extends AbstractIndexComponent {
         });
 
         Map<String, TriFunction<Settings, Version, ScriptService, Similarity>> builtIn = new HashMap<>();
-        builtIn.put(CLASSIC_SIMILARITY,
-                (settings, version, script) -> {
-                    if (version.onOrAfter(Version.V_7_0_0)) {
-                        throw new IllegalArgumentException("The [classic] similarity may not be used anymore. Please use the [BM25] "
-                                + "similarity or build a custom [scripted] similarity instead.");
-                    } else {
-                        deprecationLogger.deprecated("The [classic] similarity is now deprecated in favour of BM25, which is generally "
-                                + "accepted as a better alternative. Use the [BM25] similarity or build a custom [scripted] similarity "
-                                + "instead.");
-                        return SimilarityProviders.createClassicSimilarity(settings, version);
-                    }
-                });
         builtIn.put("BM25",
                 (settings, version, scriptService) -> SimilarityProviders.createBM25Similarity(settings, version));
         builtIn.put("boolean",

+ 5 - 0
server/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -729,6 +729,11 @@ public class IndicesService extends AbstractLifecycleComponent
                 recoveryStats.addTotals(indexShard.recoveryStats());
             }
         }
+
+        @Override
+        public void afterIndexShardClosed(ShardId shardId, IndexShard indexShard, Settings indexSettings) {
+
+        }
     }
 
     /**

+ 1 - 4
server/src/test/java/org/apache/lucene/queries/BlendedTermQueryTests.java

@@ -40,8 +40,6 @@ import org.apache.lucene.search.ScoreMode;
 import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.search.TopDocs;
 import org.apache.lucene.search.similarities.BM25Similarity;
-import org.apache.lucene.search.similarities.ClassicSimilarity;
-import org.apache.lucene.search.similarities.Similarity;
 import org.apache.lucene.store.Directory;
 import org.elasticsearch.test.ESTestCase;
 
@@ -208,8 +206,7 @@ public class BlendedTermQueryTests extends ESTestCase {
     }
 
     public IndexSearcher setSimilarity(IndexSearcher searcher) {
-        Similarity similarity = random().nextBoolean() ? new BM25Similarity() : new ClassicSimilarity();
-        searcher.setSimilarity(similarity);
+        searcher.setSimilarity(new BM25Similarity());
         return searcher;
     }
 

+ 2 - 2
server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -800,9 +800,9 @@ public class ScopedSettingsTests extends ESTestCase {
         assertEquals("Failed to parse value [true] for setting [index.number_of_replicas]", e.getMessage());
 
         e = expectThrows(IllegalArgumentException.class, () ->
-            settings.validate("index.similarity.classic.type", Settings.builder().put("index.similarity.classic.type", "mine").build(),
+            settings.validate("index.similarity.boolean.type", Settings.builder().put("index.similarity.boolean.type", "mine").build(),
                 false));
-        assertEquals("illegal value for [index.similarity.classic] cannot redefine built-in similarity", e.getMessage());
+        assertEquals("illegal value for [index.similarity.boolean] cannot redefine built-in similarity", e.getMessage());
     }
 
     public void testValidateSecureSettings() {

+ 2 - 2
server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java

@@ -375,7 +375,7 @@ public class GatewayIndexStateIT extends ESIntegTestCase {
         final IndexMetaData brokenMeta = IndexMetaData.builder(metaData).settings(Settings.builder().put(metaData.getSettings())
                 .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT.minimumIndexCompatibilityVersion().id)
                 // this is invalid but should be archived
-                .put("index.similarity.BM25.type", "classic")
+                .put("index.similarity.BM25.type", "boolean")
                 // this one is not validated ahead of time and breaks allocation
                 .put("index.analysis.filter.myCollator.type", "icu_collation")
         ).build();
@@ -397,7 +397,7 @@ public class GatewayIndexStateIT extends ESIntegTestCase {
 
         state = client().admin().cluster().prepareState().get().getState();
         assertEquals(IndexMetaData.State.CLOSE, state.getMetaData().index(metaData.getIndex()).getState());
-        assertEquals("classic", state.getMetaData().index(metaData.getIndex()).getSettings().get("archived.index.similarity.BM25.type"));
+        assertEquals("boolean", state.getMetaData().index(metaData.getIndex()).getSettings().get("archived.index.similarity.BM25.type"));
         // try to open it with the broken setting - fail again!
         ElasticsearchException ex = expectThrows(ElasticsearchException.class, () -> client().admin().indices().prepareOpen("test").get());
         assertEquals(ex.getMessage(), "Failed to verify index " + metaData.getIndex());

+ 0 - 15
server/src/test/java/org/elasticsearch/index/similarity/SimilarityTests.java

@@ -63,21 +63,6 @@ public class SimilarityTests extends ESSingleNodeTestCase {
         assertThat(similarityService.getSimilarity("BM25").get(), instanceOf(LegacyBM25Similarity.class));
         assertThat(similarityService.getSimilarity("boolean").get(), instanceOf(BooleanSimilarity.class));
         assertThat(similarityService.getSimilarity("default"), equalTo(null));
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-                () -> similarityService.getSimilarity("classic"));
-        assertEquals("The [classic] similarity may not be used anymore. Please use the [BM25] similarity or build a custom [scripted] "
-                + "similarity instead.", e.getMessage());
-    }
-
-    public void testResolveSimilaritiesFromMapping_classicIsForbidden() throws IOException {
-        Settings indexSettings = Settings.builder()
-            .put("index.similarity.my_similarity.type", "classic")
-            .put("index.similarity.my_similarity.discount_overlaps", false)
-            .build();
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-                () -> createIndex("foo", indexSettings));
-        assertEquals("The [classic] similarity may not be used anymore. Please use the [BM25] similarity or build a custom [scripted] "
-                + "similarity instead.", e.getMessage());
     }
 
     public void testResolveSimilaritiesFromMapping_bm25() throws IOException {