Upgrading an index using a custom similarity fails

[xabbu42]

Elasticsearch version: 5.1.1 - 5.4.1

Plugins installed: [analysis-icu, elasticsearch-position-similarity]

JVM version (java -version):
openjdk version "1.8.0_131"
OpenJDK Runtime Environment (IcedTea 3.4.0) (suse-10.10.3-x86_64)
OpenJDK 64-Bit Server VM (build 25.131-b11, mixed mode)

OS version (uname -a if on a Unix-like system):
Linux oviken 4.4.36-8-default #1 SMP Fri Dec 9 16:18:38 UTC 2016 (3ec5648) x86_64 x86_64 x86_64 GNU/Linux

Description of the problem including expected versus actual behavior:
Upgrading an index using a custom similarity fails in org.elasticsearch.cluster.metadata.MetaDataIndexUpgradeService.checkMappingsCompatibility with the exception "Unknown Similarity type". If the old and the new elasticserver have the same custom similarity installed, upgrading should work.

Looking at the code, it is no surprise that this does not work, as checkMappingsCompatibility creates a SimilarityService with an empty list of additional similarities. It seems something akin to the fake analyzerMap is necessary for custom similarities?

This is a follow up to https://discuss.elastic.co/t/upgrading-index-with-custom-similarity-not-working/76678 as by now I'm quite sure this is a bug with elasticsearch an not with my similarity plugin.

Steps to reproduce:

1. Start elasticsearch version 5.1.1 with https://github.com/sdauletau/elasticsearch-position-similarity installed (I've attached the exact zip I build and used)
   elasticsearch-position-similarity-5.1.1.zip

2. Create an index using this similarity:

curl -s -XPUT "http://localhost:9200/test_index" -d '
{
  "settings": {
    "similarity": {
      "positionSimilarity": {
        "type": "position-similarity"
      }
    }
  }
}'

3. Stop elasticsearch, upgrade to 5.1.2, install the same plugin for this version (I again attached the zip I used)
   elasticsearch-position-similarity-5.1.2.zip

4. Start elasticsearch again

Provide logs (if relevant):

elasticsearch.txt

https://gist.github.com/xabbu42/66dd46bd3cc15244cede8ceadf884fd8

[rjernst]

This looks like a bug in how similarities are registered. Currently they are added inside Plugin.onIndexModule. But this is too late when the node is starting up with existing similarites in index settings.

I think we should move similarities out to a pull based plugin interface, and have the index get a registry of similarity impls. @s1monw wdyt?

[seekely]

We ran into the same issue when trying to upgrade from 2.3 to 5.2 (both in place and from snapshot). We were thinking of patching and building ourselves, but a proper fix did not look very straight forward to our unfamiliar eyes since the similarities are not loaded until an IndexService is created and that looked a little tough to just mock/fake in this scenario.

[xabbu42]

I was also planning to try a patch myself using a similar workaround as is already done with analyzers in checkMappingsCompatibility. But the comments of rjernst suggest that the problem is more complex and so I'm waiting on further input before trying anything myself.

[rjernst]

@xabbu42 I think your idea of using a similar workaround as we do with analyzers is the correct solution. My earlier comment was missing some context and would be something nice to have, but is a separate issue.

[pweerd]

In general, I would advocate a settings that allows an index with a problematic custom similarity to be loaded anyway (with kind of a no-op similarity). This enables us to manipulate the index-definition afterwards.

[thomas11]

@rjernst I'd like to have this fix in the 5.x branch as well. I've ported it and it works for me locally. Can I open a PR?