powerhome / rails-components-multi-database

0 stars 0 forks source link

Database config lookup doesn't work #1

Open benlangfeld opened 5 years ago

benlangfeld commented 5 years ago
{18:07}~/code/rails-components-multi-database/components/destinations:master ✗ [2.5.0]
➭ bin/rails c

From: /Users/blangfeld/code/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @ line 952 ActiveRecord::ConnectionAdapters::ConnectionHandler#establish_connection:

    950: def establish_connection(config)
    951:   resolver = ConnectionSpecification::Resolver.new(Base.configurations)
 => 952:   binding.pry
    953:   spec = resolver.spec(config)
    954:
    955:   remove_connection(spec.name)
    956:
    957:   message_bus = ActiveSupport::Notifications.instrumenter
    958:   payload = {
    959:     connection_id: object_id
    960:   }
    961:   if spec
    962:     payload[:spec_name] = spec.name
    963:     payload[:config] = spec.config
    964:   end
    965:
    966:   message_bus.instrument("!connection.active_record", payload) do
    967:     owner_to_pool[spec.name] = ConnectionAdapters::ConnectionPool.new(spec)
    968:   end
    969:
    970:   owner_to_pool[spec.name]
    971: end

[1] pry(#<ActiveRecord::ConnectionAdapters::ConnectionHandler>)> config
=> {:adapter=>"mysql2", :encoding=>"utf8", :pool=>5, :username=>"root", :password=>"foo", :host=>"127.0.0.1", :database=>"destinations_development", :name=>"primary"}
[2] pry(#<ActiveRecord::ConnectionAdapters::ConnectionHandler>)> exit

From: /Users/blangfeld/code/rails/activerecord/lib/active_record/connection_adapters/connection_specification.rb @ line 163 ActiveRecord::ConnectionAdapters::ConnectionSpecification::Resolver#spec:

    158:         def spec(config)
    159:           pool_name = config if config.is_a?(Symbol)
    160:
    161:           spec = resolve(config, pool_name).symbolize_keys
    162:
 => 163:           binding.pry
    164:
    165:           raise(AdapterNotSpecified, "database configuration does not specify adapter") unless spec.key?(:adapter)
    166:
    167:           # Require the adapter itself and give useful feedback about
    168:           #   1. Missing adapter gems and

[1] pry(#<ActiveRecord::ConnectionAdapters::ConnectionSpecification::Resolver>)> exit
Loading development environment (Rails 6.0.0.alpha)
>> Destinations::Destination.first

From: /Users/blangfeld/code/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @ line 952 ActiveRecord::ConnectionAdapters::ConnectionHandler#establish_connection:

    950: def establish_connection(config)
    951:   resolver = ConnectionSpecification::Resolver.new(Base.configurations)
 => 952:   binding.pry
    953:   spec = resolver.spec(config)
    954:
    955:   remove_connection(spec.name)
    956:
    957:   message_bus = ActiveSupport::Notifications.instrumenter
    958:   payload = {
    959:     connection_id: object_id
    960:   }
    961:   if spec
    962:     payload[:spec_name] = spec.name
    963:     payload[:config] = spec.config
    964:   end
    965:
    966:   message_bus.instrument("!connection.active_record", payload) do
    967:     owner_to_pool[spec.name] = ConnectionAdapters::ConnectionPool.new(spec)
    968:   end
    969:
    970:   owner_to_pool[spec.name]
    971: end

[1] pry(#<ActiveRecord::ConnectionAdapters::ConnectionHandler>)> config
=> {:database=>"destinations", :name=>"Destinations::ApplicationRecord"}
[2] pry(#<ActiveRecord::ConnectionAdapters::ConnectionHandler>)> resolver
=> #<ActiveRecord::ConnectionAdapters::ConnectionSpecification::Resolver:0x00007fc144c5efb0
 @configurations=
  #<ActiveRecord::DatabaseConfigurations:0x00007fc144a86468
   @configurations=
    [#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fc144a84a78
      @config={"adapter"=>"mysql2", "encoding"=>"utf8", "pool"=>5, "username"=>"root", "password"=>"foo", "host"=>"127.0.0.1", "database"=>"destinations_development"},
      @env_name="development",
      @spec_name="primary">,
     #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fc144a849b0
      @config={"adapter"=>"mysql2", "encoding"=>"utf8", "pool"=>5, "username"=>"root", "password"=>"foo", "host"=>"127.0.0.1", "database"=>"destinations_development"},
      @env_name="development",
      @spec_name="destinations">,
     #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fc144a84898
      @config={"adapter"=>"mysql2", "encoding"=>"utf8", "pool"=>5, "username"=>"root", "password"=>"foo", "host"=>"127.0.0.1", "database"=>"destinations_test"},
      @env_name="test",
      @spec_name="primary">,
     #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fc144a84618
      @config={"adapter"=>"mysql2", "encoding"=>"utf8", "pool"=>5, "username"=>"root", "password"=>"foo", "host"=>"127.0.0.1", "database"=>"destinations_production"},
      @env_name="production",
      @spec_name="primary">]>>
[3] pry(#<ActiveRecord::ConnectionAdapters::ConnectionHandler>)> exit

From: /Users/blangfeld/code/rails/activerecord/lib/active_record/connection_adapters/connection_specification.rb @line 163 ActiveRecord::ConnectionAdapters::ConnectionSpecification::Resolver#spec:

    158:         def spec(config)
    159:           pool_name = config if config.is_a?(Symbol)
    160:
    161:           spec = resolve(config, pool_name).symbolize_keys
    162:
 => 163:           binding.pry
    164:
    165:           raise(AdapterNotSpecified, "database configuration does not specify adapter") unless spec.key?(:adapter)
    166:
    167:           # Require the adapter itself and give useful feedback about
    168:           #   1. Missing adapter gems and

[1] pry(#<ActiveRecord::ConnectionAdapters::ConnectionSpecification::Resolver>)> exit
Traceback (most recent call last):
        1: from (irb):1
ActiveRecord::AdapterNotSpecified (database configuration does not specify adapter)
>> exit
benlangfeld commented 5 years ago

For now it does not tackle any configuration changes (we can hash that out in future PRs).

https://github.com/rails/rails/pull/34052#issue-219792611

terryfinn commented 5 years ago

@benlangfeld Have you tried using the establish_connection method to pick the database to connect to in the base records instead of the connects_to method? This is what @eileencodes does in her example application. Here database.yml looks like this, and her base records looks like this and this.

eileencodes commented 5 years ago

1) The comment about configurations was specifically regarding DHH wanting more levels than 2 or 3 tier configs. Configurations work fine otherwise.

2) The demo app that @terryfinn is super old and not correct.

3) I don't know if the way you're using connects_to can work like that. That connects_to is per model so that the connection_specification_name can be applied correctly. You can't put it in ApplicationRecord (every single connection would then get ApplicationRecord as it's connection_specification_name and they'd each clobber each other). You need a model per database type like @terryfinn linked to in https://github.com/eileencodes/multiple_databases_demo/blob/master/app/models/animals_base.rb

benlangfeld commented 5 years ago

@terryfinn Yes, I did try that, with the same result.

@eileencodes Thanks for commenting! We do indeed have a different base class for each database type, being an Airlines::ApplicationRecord and a Destinations::ApplicationRecord, where each live in separate engines. Would that not be enough for the specification name not to collide?

The intent here is for the set of models/tables in each engine (which are combined into a single application) to use the same database (2 engines mounted in an app therefore using two databases). For context, this is an exploration of how Rails 6' multi-database support might work with Component Based Rails Apps. Our real-world use case is of course significantly more complex.

eileencodes commented 5 years ago

Thanks for commenting! We do indeed have a different base class for each database type, being an Airlines::ApplicationRecord and a Destinations::ApplicationRecord, where each live in separate engines. Would that not be enough for the specification name not to collide?

I can't tell from your database config but it looks like maybe you're implementing sharding? We haven't implemented that at all yet, we just have basic connection switching for telling the code to go from a primary to a replica and back again:

The problem is here:

https://github.com/powerhome/rails-components-multi-database/blob/8efc64bea66394e035fa92dbee03b7ac9b23f389/components/destinations/app/models/destinations/application_record.rb#L7

All of the connections you create here will have Destinations::ApplicationRecord as the specification name. The last one will win so you'll only have a writing and reading connection for the last one in the db_name list.

benlangfeld commented 5 years ago

Yes, the intent is sharding. I guess I mis-understood what had been implemented so far. Is support for sharding on the roadmap for Rails 6.0?

As for the "last one wins", we're not actually looping there so I don't think that's a concern.

Thank you again for your help, super-hero style!

eileencodes commented 5 years ago

As for the "last one wins", we're not actually looping there so I don't think that's a concern.

Ah right, looked like you were for some reason. Long day! 😄

Is support for sharding on the roadmap for Rails 6.0?

Yea we plan on implementing it for 6. There are folks on the core team who use sharding in production that are interested in seeing it go in. We don't use sharding at GH so I'm focusing on getting other stuff working.

terryfinn commented 5 years ago

@benlangfeld I was able to modify your branch and was able to get the models connecting to the database. I made three changes:

  1. Removed the conditional logic that was looking for a db_name_file under the Rails.root. This file only seemed to be present in the engines dummy app folder. Not sure what the use was of that, but it was not available in the context I was running things form.
  2. Modified the components ApplicationRecord classes to use establish_connection instead of connects_to.
  3. Modified the config/database.yml to enumerate the configurations for the different components databases.
 module Airlines
   class ApplicationRecord < ActiveRecord::Base
     self.abstract_class = true
-    db_name_file = File.join(Rails.root, "db/name")
-    if File.exists?(db_name_file)
-      db_name = File.read(db_name_file).chomp
-      establish_connection db_name
-    end
+    establish_connection :airline
   end
 end
 development:
   primary:
     <<: *default
     database: <%= db_name %>_development
-  <%= db_name %>:
+  airline:
     <<: *default
-    database: <%= db_name %>_development
+    database: airline_development
benlangfeld commented 5 years ago

And when you run the dummy app, does that work, or only the umbrella? It’s important that it works in the dummy app so that we can run tests.

The “routes” component is intended to depend on the other two and implement a join table, demonstrating how tests would run concurrently with such dependencies, where the routes component would need 3 databases separate from the databases used by each of its dependencies.