Elasticsearch-rails: Index name is being set on repository class not instance

Created on 4 Jul 2018  路  8Comments  路  Source: elastic/elasticsearch-rails

Hey, I've found out some weird behaviour, want to clarify I'm using the gem correctly. Here is the repository I have -

class UserRepository
  include Elasticsearch::Persistence::Repository

  klass User

  def initialize index_name: nil;
    type TYPE
    client Elasticsearch::Client.new url: Figaro.env.elasticsearch_url!, log: configatron.elasticsearch.log
    index index_name || DEFAULT_INDEX_NAME

    create_index! unless index_exists?
  end
end

Now I want to write to a specific index using this repository. So I create a repository and provide an index name in arguments

> default_index_repo = UserRepository.new
> default_index_repo.index
=> "development_users"
> custom_index_repo = UserRepository.new(index_name: 'custom_index_name')
> custom_index_repo.index
=> "custom_index_name"

Everything seems fine so far. But then I try following code

> default_index_repo.index
=> "custom_index_name"
> UserRepository.index
=> "custom_index_name"

Now all repositories that I created before are pointing to a different index. This is something I didn't expect. Is this behaviour designed?

The problem is that I run such code in multithreaded environment(puma) and I need to create repositories that work with different indices. But when I create two repositories with different indices in different threads I have no idea what index they will actually use. Can someone advice?

Most helpful comment

Hi @fatbeard2 Thanks for posting the code that worked for you. I think what's best for now is if we keep this issue open so we can investigate if it makes more sense to update the documentation or actual behavior of the Repository mixin.

All 8 comments

@fatbeard2 The various settings on a Repository are set at the class-level, so having two instances of the UserRepository with different settings might cause some issues. I would suggest defining two separate classes for your scenario. Please let me know if that works for you. Thanks!

Ok so I was able to solve the issue using different class, it requires to have extra builder service to actually build this repository with proper index, but it works. Here are some pieces of code that I came up with when trying to find out a working solution.

class UserRepositoryBuilder
  def self.build index_name: nil
    index_name ? custom_index_repository(index_name) : UserRepository
  end

  def self.custom_index_repository index_name
    Class.new do # I think Elasticsearch::Persistence::Repository.new will also work
      include Elasticsearch::Persistence::Repository
      include SharedRepositoryMethods
      index index_name
      type :user
      #...
    end
  end
end

But documentation suggests to use user defined repository class. It also mentions that you're able to set a custom index in initialize

def initialize(options={})
    index  options[:index] || 'notes'

But if you run such code in a multithreaded environment(or a simpler case when you try to use two repositories with different indices at the same time) you'll get unexpected behaviour. Or even subtle production bugs that you may debug for week.

Do you think its worth mentioning this behaviour in documentation?

Hi @fatbeard2 Thanks for posting the code that worked for you. I think what's best for now is if we keep this issue open so we can investigate if it makes more sense to update the documentation or actual behavior of the Repository mixin.

@fatbeard2 I've opened a pull request against the 6.x branch that changes the way the repository pattern should be used with this gem. With the changes proposed in the pull request, you will avoid the issues that are reported here. Please let me know if you have any feedback, thanks.

@fatbeard2 I've opened another pull request to refactor the gem in a different way, in case you want to look at that one as well.

@fatbeard2, 6.0.0.pre versions are available of elasticsearch-rails, elasticsearch-model, and elasticsearch-persistence. They include a refactor so that this problem should be resolved. Feel free to try it out in your testing environment.

Closing this issue, as the refactor has been merged and a pre gem version is released.

Was this page helpful?
0 / 5 - 0 ratings