Model's settings and mappings are ignored when the index is created through Model.import

[dimroc]

Hi all,
I'm trying to use mappings defined in an AR model backed by postgres (rails 4.0.4) to create an index and its corresponding mappings. My model has the following:

    mappings dynamic: false do
      indexes :text
      indexes :neighborhood, index: 'not_analyzed'
    end

Unfortunately, when I run curl -XGET 'http://localhost:9200/my_index/_mapping?pretty=true'

{
  "my_index" : {
    "mappings" : {
      "my_type" : {
        "properties" : {
          "index" : {
            "properties" : {
              "_id" : {
                "type" : "long"
              },
              "_score" : {
                "type" : "long"
              },
              "neighborhood" : {
                "type" : "string"
              },
              "text" : {
                "type" : "string"
              }
            }
          }
        }
      }
    }
  }
}

Notice the lack of "index": "not_analyzed". Using the curl commands against ES directly, I was able to create the index needed.

This is all invoked using ModelName.import.

[dimroc]

All settings and mappings are ignored by Model.import unless one passes in force: true.

Model.import force: true

As shown by this line of code

Makes sense since the import can be run on existing indices. Perhaps explicitly checking if the index does not exist before running create_index! with the configured settings and mappings could be a solution.

It's a shame that the first run of Model.import will ignore a models configuration. If all agree on the above, I'll go ahead and implement it and submit a pull request.

hello dimroc, I'm experiencing the same issue myself. I had some columns defined as decimal on my postgres database and after Model.import or Model.import force: true the resulting mapping in elasticsearch was string.

Furthermore, at least Integer and dates are being correctly mapped.
Using Rails 4.1.4, elasticsearch 1.2.1 and latest elasticsearch-rails/model

[karmi]

@dimroc No, you're mistaken. The create_index! will create the index with correct mappings and settings, you should check these lines of code...

Evidence:

> Article.import force: true

# DELETE http://localhost:9200/articles [status:200, request:0.021s, query:n/a]
# < {"acknowledged":true}
# ...
# PUT http://localhost:9200/articles [status:200, request:0.229s, query:n/a]
# > {"settings":{},"mappings":{"article":{"properties":{}}}}
# < {"acknowledged":true}

See the the activerecord_article.rb in examples.

---

@Hugobento Setting properties in Postgres doesn't necessarily propagate anything into the Elasticsearch mapping. You should open a separate issue for your problem, if you want to discuss it.

[dimroc]

Hi @karmi, I mentioned that in my second comment: It will work if you pass force: true.

My point is that running Article.import without force:true while not having an existing index will erroneously create an index with the default values (type: "string"). create_index! is never actually invoked, the index is created lazily by client.bulk.

You can see the if check right here.

I'm proposing that Article.import explicitly check if the index exists, and invoke create_index! if it doesn't.

Something like this:

if options.delete(:force) || index_does_not_exist(target_index)
  self.create_index! force: true, index: target_index
end

[karmi]

I think one of my comments got lost -- was travelling the last days.

I'm proposing that Article.import explicitly check if the index exists, and invoke create_index! if it doesn't.

This cannot work like this, in my mind: consider the case where you have two models, with different mappings, within one index. See https://github.com/elasticsearch/elasticsearch-rails/blob/master/elasticsearch-persistence/examples/music/index_manager.rb#L10-L16 for an example.

---

UPDATE: And before you mention updating the mapping for the second model, sorry, I simply won't go there :) The linked IndexManager is a much more sane approach, in my opinion.

[dimroc]

Do we want to silently create an index with the wrong mappings though? Because that's what we're doing now if we naively call Article.import with mappings but without an index, which is what a lot of new users will probably do. My main goal here is to prevent introductory users from creating the wrong mappings without knowing it. Perhaps it's as simple as raising an error if the index doesn't exist.

RE: IndexManager, I completely agree that having the separate IndexManager is the right approach for your scenario: One index with two types, one for each model. elasticsearch-model doesn't need to handle that out of the box, the developer should handle that explicitly and not expect import to handle it.

The change I mentioned doesn't change that. Because invoking SecondModel.import would be harmless since the index would already exist after running FirstModel.import and create_index! force: true never gets invoked. But, now the SecondModel's mappings are busted and we have the same problem all over again...

So, now I am leaning towards raising an error if mappings exist but an index doesn't. Your call @karmi! Let me know what you think.

[karmi]

My main goal here is to prevent introductory users from creating the wrong mappings without knowing it.

I don't believe, after multiple years working in open source, that you can prevent user error and confusion :)

So in my opinion, import should do what it says on the tin: import the data. Notice that the documentation at http://rubydoc.info/gems/elasticsearch-model/Elasticsearch/Model/Importing/ClassMethods#import-instance_method quite explicitly lists this example, Delete and create the index with appropriate settings and mappings.

So, now I am leaning towards raising an error (...)

Not an error, but possibly a warning. The really problematic case is still different though -- mappings in the existing index are different than those defined in the model, maybe because the user have updated them, without recreating the index.

I'm not strongly against such feedback to the user, but don't consider it something we should spend time on -- there are many other more important areas of improvement. That said, I'm all for opening an issue/PR for that particular problem, “Display warning when mapping in the index differs from mapping in the model during import”.

[dimroc]

Alright, if it's not a serious concern, I'll punt on it. Good talk. Closing issue.

[nanaya]

just pinging in: from what I can tell, create_index! doesn't really do anything if the index already exists... I even have it in initializer to prevent people from having wrong index mapping when doing import.

Something like this in previously-discussed importing.rb

self.create_index! force: options.delete(:force), index: target_index

(yeah, I've been bit by this)

[denispeplin]

This can be VERY confusing. Spent few hours trying to figure out what's going on on production after removing and re-creating wrongly mapped index. Filters stopped working.
Warning will not help when import will start to put tons of logs into console.

[karmi]

@denispeplin I understand the frustration, and feel for you -- but read the discussion, there are no easy ways out of this situation..

When re-reading the discussion with @dimroc, I think it's maybe reasonable for Model.import to raise an exception when the index doesn't exist -- sorry, Dimitri, for not seeing that clearly the first time. That's something we actually should do -- I'll reopen the issue.