influitive / apartment

Database multi-tenancy for Rack (and Rails) applications
2.67k stars 463 forks source link

Multi-proc Multi-threaded puma web server on Heroku not switching tenants properly #576

Closed bpirtle-nova closed 5 years ago

bpirtle-nova commented 5 years ago

Steps to reproduce

Inside a rails controller:

around_action :isolate

def isolate
    Apartment::Tenant.switch(get_current_user.get_schema) do
      begin
        yield
      rescue => err
        pp "[Isolation Error] #{err}"
      end
    end

    #Just in case we didn't switch back to the default due to an internal failure
    pp "Apartment tenant before icm api reset:#{Apartment::Tenant.current}"
    pp "People model before icm api entry reset:#{Person.table_name}"
    Apartment::Tenant.switch {}
    pp "Apartment tenant after icm api reset:#{Apartment::Tenant.current}"
    pp "People model after icm api entry reset:#{Person.table_name}"
  end

Expected behavior

Switch to the desired schema for the named tenant in the Apartment::Tenant.switch block and then reset back to public after the block completes. I have 2 controllers, and one uses tenant switching, while the other only uses public schema, so I do not use Apartment at all for that api.

Actual behavior

The Active Record models permanently keep the schema in Model.table_name from the last tenant and do not go back to the public schema. The pp statements from above show in the server logs that the schema is not being properly reset to public, even after the final call to Apartment::Tenant.switch {}. The only way I am able to get this to work is to manually reset the table_name for each model (i.e. Person.table_name = 'public.people') after the block finishes.

I discovered this when some requests would go to the Apartment-enabled tenant-switching controller (Using the above code in an around action) and then immediately afterward another request would use that same thread on tanother controller which does not use Apartment at all, and all models' table_name would not be back to public (It'd still be on the prior tenant's schema from the other api!)

System configuration

This is a heroku-based multi-threaded, multi-worker web app using puma, with puma config:

workers Integer(ENV['WEB_CONCURRENCY'] || 3)
threads_count = Integer(ENV['MAX_THREADS'] || 10)
threads threads_count, threads_count

preload_app!

on_worker_boot do
  # Worker specific setup for Rails 4.1+
  # See: https://devcenter.heroku.com/articles/deploying-rails-applications-with-the-puma-web-server#on-worker-boot
  ActiveRecord::Base.establish_connection
end

#

Apartment Configuration

# Apartment.configure do |config|

Add any models that you do not want to be multi-tenanted, but remain in the global (public) namespace.

A typical example would be a Customer or Tenant model that stores each Tenant's information.

# config.excluded_models = %w{ User Team Sequence Invite Utility SalesforceMetadata }

In order to migrate all of your Tenants you need to provide a list of Tenant names to Apartment.

You can make this dynamic by providing a Proc object to be called on migrations.

This object should yield either:

- an array of strings representing each Tenant name.

- a hash which keys are tenant names, and values custom db config (must contain all key/values required in database.yml)

#

config.tenant_names = lambda{ Customer.pluck(:tenant_name) }

config.tenant_names = ['tenant1', 'tenant2']

config.tenant_names = {

'tenant1' => {

adapter: 'postgresql',

host: 'some_server',

port: 5555,

database: 'postgres' # this is not the name of the tenant's db

but the name of the database to connect to before creating the tenant's db

mandatory in postgresql

},

'tenant2' => {

adapter: 'postgresql',

database: 'postgres' # this is not the name of the tenant's db

but the name of the database to connect to before creating the tenant's db

mandatory in postgresql

}

}

config.tenant_names = lambda do

Tenant.all.each_with_object({}) do |tenant, hash|

hash[tenant.name] = tenant.db_configuration

end

end

# config.tenant_names = lambda { Team.where.not(:schema => nil).pluck :schema }

PostgreSQL:

Specifies whether to use PostgreSQL schemas or create a new database per Tenant.

#

MySQL:

Specifies whether to switch databases by using use statement or re-establish connection.

#

The default behaviour is true.

#

config.use_schemas = true

#

==> PostgreSQL only options

Apartment can be forced to use raw SQL dumps instead of schema.rb for creating new schemas.

Use this when you are using some extra features in PostgreSQL that can't be represented in

schema.rb, like materialized views etc. (only applies with use_schemas set to true).

(Note: this option doesn't use db/structure.sql, it creates SQL dump by executing pg_dump)

#

config.use_sql = false

There are cases where you might want some schemas to always be in your search_path

e.g when using a PostgreSQL extension like hstore.

Any schemas added here will be available along with your selected Tenant.

#

config.persistent_schemas = %w{ hstore }

<== PostgreSQL only options

#

By default, and only when not using PostgreSQL schemas, Apartment will prepend the environment

to the tenant name to ensure there is no conflict between your environments.

This is mainly for the benefit of your development and test environments.

Uncomment the line below if you want to disable this behaviour in production.

#

config.prepend_environment = !Rails.env.production?

When using PostgreSQL schemas, the database dump will be namespaced, and

apartment will substitute the default namespace (usually public) with the

name of the new tenant when creating a new tenant. Some items must maintain

a reference to the default namespace (ie public) - for instance, a default

uuid generation. Uncomment the line below to create a list of namespaced

items in the schema dump that should not have their namespace replaced by

the new tenant

#

config.pg_excluded_names = ["uuid_generate_v4"]

end

Setup a custom Tenant switching middleware. The Proc should return the name of the Tenant that

you want to switch to.

Rails.application.config.middleware.use Apartment::Elevators::Generic, lambda { |request|

request.host.split('.').first

}

Rails.application.config.middleware.use Apartment::Elevators::Domain

Rails.application.config.middleware.use Apartment::Elevators::Subdomain

Rails.application.config.middleware.use Apartment::Elevators::FirstSubdomain

Rails.application.config.middleware.use Apartment::Elevators::Host



  * `use_schemas`: (`true` or `false`) true

* Rails (or ActiveRecord) version: Rails 4.2.10

* Ruby version: 2.1.3
bpirtle-nova commented 5 years ago

Can anyone give me any feedback as to whether my setup looks like it should be working as I expect or if there is an obvious setup issue?

I am new to this gem so might have something unintentionally bungled and I'd like to rule it out early if at all possible!

bpirtle-nova commented 5 years ago

Our mistake, we were switching the model's table_name outside of Apartment and not switching back. Once we removed that and rely completely on Apartment, everything works as expected!