rails-on-services / apartment

Database multi-tenancy for Rack (and Rails) applications
387 stars 159 forks source link

Concurrency problems in specs #239

Open dbil25 opened 11 months ago

dbil25 commented 11 months ago

Steps to reproduce

In rspec, especially in feature specs, multiple threads can run concurrently ( for example the test itself and the test server) but they both use the same ActiveRecord database connection. Thats makes it that when a spec changes the schema by doig a SET search_path TO, then it can also affects the other threads causing unexpected behaviors. At least thats what i understood after debugging some very flaky behavior in the feature specs.

This does not happen in a normal context (development and production), it only happens during specs, especially feature specs.

Is there an existing solution for this kind of bugs? meanwhile, if other people encounter the same problem i have a working fix by adding a monkey-patch in a spec/support file:

# patch to fix concurrency problem in test mode. In test mode, you can have multiple threads
# that uses the same ActiveRecord connection. Each thread has its own instance of apartment
# and can do a Tenant.switch!. This cause other threads to start using the wrong schema.
# with this patch, before every single sql query, we validate that we are doing it in the
# correct schema. If we are not in the correct schema, we switch. This is not optimal and causes
# a lot of overhead, but luckily its a test-only problem.

module ApartmentExtension
  def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false)
    raise unless Rails.env.test? # this patch is for tests only, do not use in dev/prod!
    @lock.synchronize do
      current_schema_name = get_current_schema
      if current_schema_name && current_schema_name != Apartment::Tenant.current
        Apartment::Tenant.switch!(Apartment::Tenant.current)
      end
      super(sql, name, binds, prepare: prepare, async: async)
    end
  end

  def get_current_schema
    current_schema
  rescue
    nil
  end
end

class ActiveRecord::ConnectionAdapters::PostgreSQLAdapter
  prepend ApartmentExtension
end

System configuration

dlupu commented 8 months ago

@dbil25 just run into the same issue and your solution helped me fix the issue! Thank you very much :)

otagi commented 1 month ago

I have a similar issue when upgrading Rails from 7.0 to 7.1. When running the full spec suite, rspec eventually stops without exiting (in a model spec). It runs fine with Rails 7.0.

The main thread backtrace reports:

/.../.rbenv/versions/3.3.4/lib/ruby/gems/3.3.0/gems/activesupport-7.1.4.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:55:in `lock'
# ...
/.../.rbenv/versions/3.3.4/lib/ruby/3.3.0/forwardable.rb:240:in `switch!'
/.../spec/support/apartment.rb:22:in `block (2 levels) in <main>'
# spec/support/apartment.rb

RSpec.configure do |config|
  config.before(:each) do |example|
    Apartment::Tenant.switch! @tenant_name
  end
end
# active_support/concurrency/load_interlock_aware_monitor.rb

def mon_enter
  @mutex.lock if @owner != Thread.current
  # …
end

The PostgreSQLAdapter patch above does not resolve the lock.

System configuration