Mapping deletes are no longer supported by Elasticsearch 2.3
https://www.elastic.co/guide/en/elasticsearch/reference/2.3/indices-dele...

Generates an error like so

{"found":false,"_index":"name_of_index","_type":"_mapping","_id":"id_of_index","_version":2,"_shards":{"total":2,"successful":1,"failed":0}}

Found this issue when installing elasticsearch_connector with cluster, server and index settings provided by default config.

CommentFileSizeAuthor
#4 interdiff-2795039-1-4.txt1.38 KBthursday_bw
#4 elasticsearch_connector-delete_mapping_error_with_es-2.3-2795039-4-8.x0-2.x-dev.patch1.28 KBthursday_bw
#2 elasticsearch_connector-delete_mapping_error_with_es-2.3-2795039-1-8.x02.x-dev.patch1.04 KBthursday_bw
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

thursday_bw created an issue. See original summary.

thursday_bw’s picture

thursday_bw CreditAttribution: thursday_bw at Catalyst IT commented
FileSize
elasticsearch_connector-delete_mapping_error_with_es-2.3-2795039-1-8.x02.x-dev.patch1.04 KB

Here is a patch that, instead of deleting the mapping, deletes the index and recreates it.
As per https://www.elastic.co/guide/en/elasticsearch/reference/2.0/indices-dele...

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for amaysim Australia Ltd. commented 
  1. +++ b/src/Plugin/search_api/backend/SearchApiElasticsearchBackend.php
@@ -296,8 +296,12 @@ class SearchApiElasticsearchBackend extends BackendPluginBase {
+          // in Elasticsearch 2.3 it is not possible to delete a mapping.

    nit: In instead of in 

  2. +++ b/src/Plugin/search_api/backend/SearchApiElasticsearchBackend.php
@@ -296,8 +296,12 @@ class SearchApiElasticsearchBackend extends BackendPluginBase {
+          // will generate an error.

    Does it generate an error or does the library raise an Exception? If the later, we could wrap the ->deleteMapping call in a try/catch block. That way on older versions the simpler delete mapping could remain. Newer versions would throw an exception, which we'd catch and then use the remove/add index route.

  3. +++ b/src/Plugin/search_api/backend/SearchApiElasticsearchBackend.php
@@ -296,8 +296,12 @@ class SearchApiElasticsearchBackend extends BackendPluginBase {
+          $this->addIndex($index);

    Should we also be doing something to trigger the reindexing of any content here too? E.g. marking all items in the index as needing reindex?

thursday_bw’s picture

thursday_bw CreditAttribution: thursday_bw at Catalyst IT commented
FileSize
elasticsearch_connector-delete_mapping_error_with_es-2.3-2795039-4-8.x0-2.x-dev.patch1.28 KB
interdiff-2795039-1-4.txt1.38 KB

Patch elasticsearch_connector-delete_mapping_error_with_es-2.3-2795039-4-8.x0-2.x-dev.patch and interdiff-2795039-1-4.txt supplied to add try catch so the that on older versions of elasticsearch the simpler delete mapping remains.

Does anyone have any input in regards to

Should we also be doing something to trigger the reindexing of any content here too? E.g. marking all items in the index as needing reindex?

kattekrab’s picture

kattekrab CreditAttribution: kattekrab at Catalyst IT for La Trobe University commented

Yeah - triggering a re-index rebuild seems like a good idea to me.

Dunno if that might have performance implications or not though?

thursday_bw’s picture

thursday_bw CreditAttribution: thursday_bw at Catalyst IT commented

I'm not sure, when in the logic flow this triggers. In the case where I saw this issue, it was during the creation of an index, and so an index rebuild wouldn't make sense; A re-index would be unnecessary and possibly, as mentioned, reduce performance.

I'm uncertain however if this same code executes under another circumstance where an re-index would be necessary.

I'm hoping the maintainer may understand the flow here better than I and can make a recommendation.

sambonner’s picture

sambonner CreditAttribution: sambonner at Catalyst IT commented

This looks good to me, fieldsUpdated() seems to only be called within addIndex() so it appears a re-index would be unnecessary in these circumstances. Wondering if an updateIndex test covering these conditions would be useful? I see there's a stub in ElasticdearchTest.php...

I won't set this to RTBC as I haven't actually tested the patch on a running D8 site.

thursday_bw’s picture

thursday_bw CreditAttribution: thursday_bw at Catalyst IT commented
Status: Active » Needs review

I have tested this, and am using this patch in a yet release running D8 site.
But, it's not up to me to RTBC my own patch.
Setting back to 'Needs review' to try and get this one moved forward.

rli’s picture

rli
Primary language English
Location Canberra
CreditAttribution: rli as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Elasticsearch 5.0 doesn't support delete mapping neither, delete and recreate index is always the way.
The patch looks good to me.
Thanks thursday_bw.

PS, the support for Elasticsearch 1.7 will stop in next Jan. We should consider ditch the deleteMapping function.

  • skek committed 5044372 on 8.x-2.x authored by thursday_bw 
    Issue #2795039 by thursday_bw: Elastic search 2.3 generates error on...
skek’s picture

skek CreditAttribution: skek commented
Status: Reviewed & tested by the community » Fixed

Thank you for the patch.
I've applied it as temporary fix because the logic should be based on the cluster version itself, not using the try catch a for this.
In the client library there is a method getServerVersion() that can be used to determinate which delete methods to be called.
I've created a separate issue to fix this in more general way, even thinking of the case where we should drop the support of the 1.x clusters: https://www.drupal.org/node/2824539

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.