Elasticsearch, Chewy, Postgres Schemas, Multi-Tenancy

[eliduke]

I have recently switched from MySQL over to Postgres with multiple schemas, and am having trouble with rake chewy:reset:all. It looks as though it is defaulting to the "public" schema, but I want to specify the schema. I am using the apartment gem, so I put this in one of my indexes:

Apartment::Tenant.switch!('tenant_name')

And that fixed the problem, but that got me thinking bigger about elasticsearch and multi-tenancy in general. Does chewy have any sort of implementation of that? If not, do you have any recommendations?

[pyromaniac]

Chewy does nothing with schemas, it uses queries provided by you. And query should already know which tenant to use.

[eliduke]

Here's the breakdown:

My app knows what tenant because it's one of the necessary headers needed for authentication with the API. So, within the app it is all the correct tenant.

When going into the rails console, we have a little bit of config code that checks for all available tenants and asks which one you want before fully loading the console. That's awesome.

When I console into our main tenant (not the standard "public" tenant), I have all the expected records in the database. But when I run the "rake chewy:reset:all", it indexes the data in the "public" scheme, instead of the scheme that the rest of the app is using. Why is that?

It seems like y'all need to add some sort of tenant config option where I can tell chewy to index the current tenant instead of the public one. Maybe it is just a config option specifying MySQL or Postgres or what-have-you, with specific adapter type stuff for the chosen database type.

Your thoughts?

[pyromaniac]

Actually you have to prepare environment for the rake chewy:reset task as well as for every other task. You may create a preparation task like setup_tenant and use it manually like rake setup_tenant[name] chewy:reset or even prepend this task with rake DSL to every rake task automatically. This will make rake tasks behavior similar to console. Chewy should know nothing about your tenants.

[yurifrl]

Can i sugest a change to index_name for something like:

def self.index_name(suggest = nil)
    prefix = Chewy.configuration[:prefix]

    prefix = prefix.call if prefix.respond_to? :call

    if suggest
      @index_name = build_index_name(suggest, prefix: prefix)
    else
      @index_name ||= begin
        build_index_name(
          name.sub(/Index\Z/, '').demodulize.underscore,
          prefix: prefix
        ) if name
      end
    end
    @index_name or raise UndefinedIndex
  end

so it's possible to do something like:

Chewy.settings = { prefix: -> { Apartment::Tenant.current } }

[eliduke]

Thanks for the suggestion, @yurifrl! That definitely got me going in the right direction. In the end, my solution involved creating a chewy monkey patch initializer and a custom rake task:

# config/initializers/chewy_multi_tenancy.rb

module Chewy
  class Index
    def self.index_name(suggest = nil)
      prefix = Apartment::Tenant.current
      if suggest
        @index_name = build_index_name(suggest, prefix: prefix)
      else
        @index_name ||= begin
          build_index_name(
            name.sub(/Index\Z/, '').demodulize.underscore,
            prefix: prefix
          ) if name
        end
      end
      @index_name or raise UndefinedIndex
    end
  end
end

# lib/tasks/elastic.rake

namespace :elastic do
  desc "resets all indexes for a given tenant ('rake elastic:reset TENANT=tenant_name')"
  task reset: :environment do
    if ENV['TENANT'].nil?
      puts "Uh oh! You didn't specify a tenant!\n"
      puts "Example: rake elastic:reset TENANT=tenant_name"
      exit
    elsif !Apartment.tenant_names.include?(ENV['TENANT'])
      puts "That tenant doesn't exist. Please choose from the following:\n"
      puts Apartment.tenant_names
      exit
    else
      Apartment::Tenant.switch!(ENV['TENANT'])
      Rake::Task['chewy:reset:all'].invoke
    end
  end
end

Since I have a completely separate test cluster we don't need to prefix our indexes with "test", so I redefined prefix with the current tenant name. As far as I can tell right now, chewy hits the index_name method every time a specific index is called. It then grabs the correct users index for the current tenant.

This solution hasn't been fully tested yet, but I wanted to throw this out there.

[yurifrl]

@eliduke glad i could help, but is very probable that it won't work properly, if @index_name is already set, i wont call the Apartment::Tenant.current again, so it will only switch the first time.

[eliduke]

@yurifrl So, does @index_name get set once per chewy index? Because I have 3 indexes and when I query each index I see in the server output that :index does indeed have a prefix of my current tenant for each separate index. I haven't tried switching tenants on the fly yet.

And so if you think that solution won't work properly, can you think of a better way? To be completely honest, I'm a little surprised that this hasn't come up yet. I guess there aren't that many folks using chewy in a multi-tenant environment.

[eliduke]

@yurifrl Just did some hand testing, switched the public tenant, queried, chewy grabbed the correct index with the prefixed tenant name and returned the correct results for the public tenant. Switched back to the previous tenant, queried again, and it returned the correct results for that tenant.

As far as I can tell, it is working just fine.

@pyromaniac Is this something that could possible by incorporated into chewy proper?

[yurifrl]

@eliduke cool, i had to use lambdas

[pyromaniac]

I don't have any objections right now, but I have to think

[eliduke]

Small update to the multi-tenant monkey patch:

# chewy multi-tenancy monkey patch
module Chewy
  class Index
    def self.index_name(suggest = nil)
      prefix = Apartment::Tenant.current
      if suggest
        @index_name = build_index_name(suggest, prefix: prefix)
      else
        @index_name = build_index_name(
            name.sub(/Index\Z/, '').demodulize.underscore,
            prefix: prefix
          ) if name
      end
      @index_name or raise UndefinedIndex
    end
  end
end

We had to remove the ||= in the else because it was remembering the index_name between different tenants and returning the wrong results. So, we just have it re-build the index_name every time and all is well.

Any more thoughts on rolling this into chewy proper?

[pyromaniac]

Yeah, but it should look like @yurifrl proposed. I mean, we have to add an ability to set up dynamic prefix, without breaking existing fuctionality

[mikeyhogarth]

@eliduke i've just submitted a PR #314 to help make your monkey patch a little smaller and easier to maintain (we've had to implement it too). After that's in, presuming it is accepted, the patch should just be:

# chewy multi-tenancy monkey patch
module Chewy
  class Index
    def self.default_prefix
      Apartment::Tenant.current
    end
  end
end

[eliduke]

That's awesome! I took a look at your changes and it would ALMOST work for what I need. In the end, I had to remove the ||= in the else because chewy wasn't switching tenants between requests. I think maybe it might be a performance thing to not rebuild the index name each time, but otherwise it wasn't working.

[eliduke]

It's a small change, but this is what I would need in your code for it to work for me:

def self.index_name(suggest = nil)
  if suggest
    @index_name = build_index_name(suggest, prefix: default_prefix)
  else
    @index_name = build_index_name(
        name.sub(/Index\Z/, '').demodulize.underscore,
        prefix: default_prefix 
      ) if name
    end
  end
  @index_name or raise UndefinedIndex
end

The ||= in your PR was causing problems for me.

[mikeyhogarth]

Not sure if they'll let me merge that though as it'll have a performance hit 😢 let's try and get the extract-method one merged in first and we can look into de-memoizing in a separate PR so it can be discussed in a more focused way.

The existing PR is still worth merging I think

[eliduke]

Absolutely! 👍

[mikeyhogarth]

Incidentally did you have any other issues getting chewy to play nice with apartment? We're literally just starting that journey 😆

[eliduke]

Not really. This issue was the big one, but I'm happy to help in any way that I can.

[mikeyhogarth]

@eliduke How did you get it to refresh all indicies for all tenants? Custom rake task?

[eliduke]

Custom rake task, INDEED!

Apartment.tenant_names.each do |tenant|
  Apartment::Tenant.switch!(tenant)
  puts "Reseting Elasticsearch Indexes for '#{tenant}' tenant..."
  Rake::Task['chewy:reset:all'].invoke
  puts "Success!"
end

And we just added that to a Heroku Scheduler task that runs every night. Along with the chewy update_index callbacks, we are pretty covered.

[mikeyhogarth]

@eliduke #318

[eliduke]

@mikeyhogarth That's awesome! Thanks for the update. I'm curious to see what the workaround is for that failing test.

[timrwilliams]

Has anybody had success using the index_name fix with the sidekiq strategy? I'm using apartment-sidekiq to add the current apartment to all sidekiq jobs on push.

My scenario is:

• Default request_strategy is set to :sidekiq
• User has an update_index callback specified on save
• User updates their record via HTTP POST. Web request correctly sets Apartment via standard Apartment middleware (other sidekiq drops triggered during request have apartment correctly set).
• When an update_index callback triggers the Chewy Sidekiq job the Apartment::Tenant.current value is not correctly set - it reverts back to the default.

[timrwilliams]

For future reference the issue here was Chewy::Railtie::RequestStrategy was being loaded before the Apartment middleware so wasn't Tenant aware.

The fix is to ensure Apartment is loaded first :

#config/initializers/apartment.rb
Rails.application.config.middleware.insert_before "Chewy::Railtie::RequestStrategy", 'Apartment::Elevators::Domain'

[silva96]

@eliduke i've just submitted a PR #314 to help make your monkey patch a little smaller and easier to maintain (we've had to implement it too). After that's in, presuming it is accepted, the patch should just be:

# chewy multi-tenancy monkey patch
module Chewy
  class Index
    def self.default_prefix
      Apartment::Tenant.current
    end
  end
end

As far as I could tell, default_prefix is now just prefix

So we should refactor the monkey patch to this:

# chewy multi-tenancy monkey patch
module Chewy
  class Index
    def self.prefix
      Apartment::Tenant.current
    end
  end
end