1. 17 9月, 2020 1 次提交
    • E
      Make `role` required when using `shard` in `connected_to` · d4df616c
      eileencodes 提交于
      While working on some more indepth changes to Rails internal connection
      management we noticed that it's confusing in some cases that the `role`
      is implcit when using a `shard`. For example, if you passed a `shard`
      and not a `role` in an un-nested block the default `role` would be
      `writing`.
      
      ```
      ActiveRecord::Base.connected_to(shard: :one) do
        # connected to writing
      end
      ```
      
      However in cases where nesting is used it could be confusing to
      application authors that the role is inherited:
      
      ```
      ActiveRecord::Base.connected_to(role: :reading) do
        ActiveRecord::Base.connected_to(shard: :one) do
          # will read from shard one replica, not write to primary
        end
      end
      ```
      
      Since this could be potentially confusing, and extremely hard to track
      in complex applications, the best approach is to require `role` when
      using `shard` which is what this PR does.
      
      Note: the code for this method is...getting unweildy. Once the
      `database` argument is fully deprecated we can remove most of the guards
      and make `role` required by removing `nil` from the keyword argument.
      Until then we need to support required arguments in this round about way.
      d4df616c
  2. 14 9月, 2020 1 次提交
  3. 14 8月, 2020 1 次提交
  4. 11 8月, 2020 1 次提交
    • E
      Fix missed establish_connection · 919eb6dc
      eileencodes 提交于
      In `connected_to` one of the deprecated arguments wasn't well tested so
      the incorrect methods signature wasn't caught by the tests.
      
      This change updates the caller when `connected_to` uses the database
      key.
      
      I've also cleaned up a few arguments that weren't necessary. Since
      the handler methods set defaults for the `shard` key, we don't need to
      pass that in `establish_connection` when not using the sharding API.
      919eb6dc
  5. 08 8月, 2020 2 次提交
    • E
      Update connection methods to use kwargs · d3061cda
      eileencodes 提交于
      This change ensures that the connection methods are using kwargs instead
      of positional arguments. This change may look unnecessary but we're
      working on refactoring connection management to make it more robust and
      flexible so the method signatures of the methods changed here will
      continue to evolve and change.
      
      This commit does not change any released public APIs. The `shard` and
      `owner_name` arguments were added in 6.1 which is not released yet.
      Using kwargs will allow these methods to be more flexible and not get
      super ugly as we change their underlying behavior. The kwargs let us
      support multiple non-positional arguments with default.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      d3061cda
    • E
      Rename `pool_key` to `shard` · 85ef2196
      eileencodes 提交于
      This change renames the following:
      
      * `current_pool_key` -> `current_shard`
      * `default_pool_key` -> `default_shard`
      * `pool_key` -> `shard`
      
      Originally we had intended to name the `shard` as `pool_key` because
      when we implemented the internal private API we weren't sure how it was
      going to be used for sharding and wanted to implement behavior without
      promising a public API. Now that we have a public API for sharding it's
      better to use the same name everywhere rather than have one name for
      private APIs and one name for public APIs. This should make
      contributions and tracking down bugs easier in the future.
      
      This PR doesn't require any deprecations because the sharding API is
      unreleased and so is all the internal code that was using `pool_key`.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      85ef2196
  6. 07 3月, 2020 2 次提交
    • E
      Remove owner_name · 7400d195
      eileencodes 提交于
      We don't actually need this since the only reason it exists is to pass
      the owning class name down to the `handler`. This removes a level of
      indirection and an unnecessary accessor on db_config. db_config
      shouldn't have to know what class owns it, so we can just remove this
      and pass it to the handler.
      
      The Symbol case is needed to preserve current behavior. This doesn't
      need a changelog because it's changing un-released behavior.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      7400d195
    • E
      Move nil config_or_env handling · 0946cb92
      eileencodes 提交于
      `config_or_env` can only be `nil` if called by `establish_connection`.
      If `connects_to` had a `connects_to database: { writing: nil }` that
      would make no sense.
      
      This is a really minor refactoring that moves the assignment if `nil`
      into the only method that can actually receive nil. Then
      `resolve_config_for_connection` will never get nil passed to it.
      0946cb92
  7. 27 2月, 2020 2 次提交
  8. 25 2月, 2020 3 次提交
    • R
    • E
      Add support for horizontal sharding · 384e7d13
      eileencodes 提交于
      Applications can now connect to multiple shards and switch between
      their shards in an application. Note that the shard swapping is
      still a manual process as this change does not include an API for
      automatic shard swapping.
      
      Usage:
      
      Given the following configuration:
      
      ```yaml
      production:
        primary:
          database: my_database
        primary_shard_one:
          database: my_database_shard_one
      ```
      
      Connect to multiple shards:
      
      ```ruby
      class ApplicationRecord < ActiveRecord::Base
        self.abstract_class = true
      
        connects_to shards: {
          default: { writing: :primary },
          shard_one: { writing: :primary_shard_one }
        }
      ```
      
      Swap between shards in your controller / model code:
      
      ```ruby
        ActiveRecord::Base.connected_to(shard: :shard_one) do
          # Read from shard one
        end
      ```
      
      The horizontal sharding API also supports read replicas. See
      guides for more details.
      
      This PR also moves some no-doc'd methods into the private namespace as
      they were unnecessarily public. We've updated some error messages and
      documentation.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      384e7d13
    • E
      Deprecate `spec_name` and use `name` for configurations · 79ce7d9a
      eileencodes 提交于
      I have so. many. regrets. about using `spec_name` for database
      configurations and now I'm finally putting this mistake to an end.
      
      Back when I started multi-db work I assumed that eventually
      `connection_specification_name` (sometimes called `spec_name`) and
      `spec_name` for configurations would one day be the same thing. After
      2 years I no longer believe they will ever be the same thing.
      
      This PR deprecates `spec_name` on database configurations in favor of
      `name`. It's the same behavior, just a better name, or at least a
      less confusing name.
      
      `connection_specification_name` refers to the parent class name (ie
      ActiveRecord::Base, AnimalsBase, etc) that holds the connection for it's
      models. In some places like ConnectionHandler it shortens this to
      `spec_name`, hence the major confusion.
      
      Recently I've been working with some new folks on database stuff and
      connection management and realize how confusing it was to explain that
      `db_config.spec_name` was not `spec_name` and
      `connection_specification_name`. Worse than that one is a symbole while
      the other is a class name. This was made even more complicated by the
      fact that `ActiveRecord::Base` used `primary` as the
      `connection_specification_name` until #38190.
      
      After spending 2 years with connection management I don't believe that
      we can ever use the symbols from the database configs as a way to
      connect the database without the class name being _somewhere_ because
      a db_config does not know who it's owner class is until it's been
      connected and a model has no idea what db_config belongs to it until
      it's connected. The model is the only way to tie a primary/writer config
      to a replica/reader config. This could change in the future but I don't
      see value in adding a class name to the db_configs before connection or
      telling a model what config belongs to it before connection. That would
      probably break a lot of application assumptions. If we do ever end up in
      that world, we can use name, because tbh `spec_name` and
      `connection_specification_name` were always confusing to me.
      79ce7d9a
  9. 29 1月, 2020 1 次提交
    • E
      Force connected_to to load the records if it's a Relation · 1d032621
      eileencodes 提交于
      Fixes #38332
      
      If the `connected_to` block returns a relation and does not inspect, or
      load that relation before returning it, the block will exit before the
      database is queried. This causes the wrong database connection to be
      queried.
      
      The consequences of this are getting records from the primary instead of
      the replica, and potentially having database performance impact.
      
      Relations lazily query the database. If you return the relation from the
      block like:
      
      ```
      posts = ActiveRecord::Base.connected_to(role: :reading) { Post.where(id: 1) }
      ```
      
      `posts.first` will be queried from the `writing` connection instead
      because it's lazy and performed outside the block. Any query that loads
      the relation (ie `to_a`) inside the block would eagerly load the
      relation's records and not exhibit this bug.
      
      `connected_to` now checks if the return value is a `Relation` and if so
      calls `load`.
      Co-authored-by: NAaron Patterson <aaron.patterson@gmail.com>
      1d032621
  10. 22 1月, 2020 1 次提交
    • E
      Deprecate `#remove_connection` in favor of `#remove_connection_pool` · 7315c91d
      eileencodes 提交于
      Calling `#remove_connection` on the handler is deprecated in favor of
      `#remove_connection_pool`. This change was made to support changing the
      return value from a hash to a `DatabaseConfig` object.
      `#remove_connection` will be removed in 6.2.
      
      NOTE: `#remove_connection` on `ActiveRecord::Base` will also now return
      a `DatabaseConfig` object. We didn't use a deprecation here since
      it's not documented that this method used to return a `Hash`.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      7315c91d
  11. 10 1月, 2020 1 次提交
    • E
      Ensure the reading connection always raises if we try to write · 1c98e6c0
      eileencodes 提交于
      Since test fixtures share connections (due to transactional tests) we
      end up overwriting the reading configuration so Rails doesn't recognize
      it as a replica connection.
      
      This change ensures that if we're using the `reading` role that
      connections will always have prevent writes turned on.
      
      If you need a replica connection that does not block writes, you should
      use a different role name other than `:reading`.
      
      The db selector test and connection handlers test have been updated to
      test for these changes. In the db selector test we don't always have a
      writing handler so I updated test fixtures to return if that's nil.
      
      Lastly one test needed to be updated to use a different handler name due
      to it needing to write to successfully test what it needs to test.
      
      Fixes #37765
      1c98e6c0
  12. 09 1月, 2020 1 次提交
    • E
      Deprecate "primary" as a connection_specification_name for ActiveRecord::Base · b74fbe4e
      eileencodes 提交于
      As multiple databases have evolved it's becoming more and more
      confusing that we have a `connection_specification_name` that defaults
      to "primary" and a `spec_name` on the database objects that defaults to
      "primary" (my bad).
      
      Even more confusing is that we use the class name for all
      non-ActiveRecord::Base abstract classes that establish connections. For
      example connections established on `class MyOtherDatabaseModel <
      ApplicationRecord` will use `"MyOtherDatabaseModel"` as it's connection
      specification name while `ActiveRecord::Base` uses `"primary"`.
      
      This PR deprecates the use of the name `"primary"` as the
      `connection_specification_name` for `ActiveRecord::Base` in favor of
      using `"ActiveRecord::Base"`.
      
      In this PR the following is true:
      
      * If `handler.establish_connection(:primary)` is called, `"primary"`
      will not throw a deprecation warning and can still be used for the
      `connection_specification_name`. This also fixes a bug where using this
      method to establish a connection could accidentally overwrite the actual
      `ActiveRecord::Base` connection IF that connection was not using a
      configuration named `:primary`.
      * Calling `handler.retrieve_connection "primary"` when
      `handler.establish_connection :primary` has never been called will
      return the connection for `ActiveRecord::Base` and throw a deprecation
      warning.
      * Calling `handler.remove_connection "primary"` when
      `handler.establish_connection :primary` has never been called will
      remove the connection for `ActiveRecord::Base` and throw a deprecation
      warning.
      
      See #38179 for details on more motivations for this change.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      b74fbe4e
  13. 18 12月, 2019 2 次提交
  14. 04 12月, 2019 1 次提交
    • E
      Deprecate `database` kwarg from `connected_to` without replacement · 254ba464
      eileencodes 提交于
      The `database` kwarg in `connected_to` has resulted in a lot of bug
      reports that are trying to use it for sharding when that's not the
      intent of the key. After considering where the database kwarg is used in
      tests and thinking about usecases for it, we've determined it should be
      removed.
      
      There are plans to add sharding support and in the mean time the
      database kwarg isn't the right solution for that. Applications that need
      to create new connections can use establish_connection or connects_to.
      Since the database key causes new connections to be established on every
      call, that causes bugs if connected_to with a database kwarg is used
      during a request or for any connection that's not a one-off.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      254ba464
  15. 13 11月, 2019 1 次提交
    • J
      Remove ConnectionAdapters::Resolver in favor of DatabaseConfigurations · 8d5a4ff6
      John Crepezzi 提交于
      We have these two objects, `ConnectionAdapters::Resolver` and
      `DatabaseConfiguratons` that implement a lot of the same logic. One of
      them is used for configurations defined in `config/database.yml` and the
      other is used when passing raw configurations `String` or `Hash` objects
      into methods like `establish_connection`.
      
      Over time these two have diverged a bit. In the interest of less code
      complexity, and more consistency for users this commit brings them back
      together.
      
      * Remove `Resolver` altogether and replace its primary method with
        `DatabaseConfigurations#resolve`.
      
      * Move `resolve_pool_config` over to the `ConnectionPool` alongside the code
        that uses it.
      8d5a4ff6
  16. 21 10月, 2019 1 次提交
  17. 24 9月, 2019 1 次提交
  18. 14 9月, 2019 2 次提交
    • E
      Reduce surface area of ConnectionSpecification · b8fc0150
      eileencodes 提交于
      Eventually we'd like to get rid of this class altogether but for now
      this PR reduces the surface area by removing methods from the class and
      moving classes out into their own files.
      
      * `adapter_method` was moved into database configurations
      * `initialize_dup` was removed because it was only used in tests
      * Resolver is now it's own class under connection adapters
      * ConnectionUrlResolver, only used by the configurations, is in a class
      under DatabaseConfigurations
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      b8fc0150
    • J
      Pass db_config object around instead of hashes · 20c7bbd4
      John Crepezzi 提交于
      This is part 1 of removing the need to pass hashes around inside
      connection management. This PR creates database config objects every
      time when passing a hash, string, or symbol and sends that object to
      connection specification. ConnectionSpecification reaches into the
      config hash on db_config when needed, but eventually we'll get rid of
      that and ConnectionSpecification since it's doing duplicate work with
      the DatabaseConfigurations.
      
      We also chose to change the keys to strings because that's what the
      database.yml would create and what apps currently expect. While symbols
      are nicer, we'd end up having to deprecate the string behavior first.
      Co-authored-by: Neileencodes <eileencodes@gmail.com>
      20c7bbd4
  19. 13 9月, 2019 1 次提交
    • E
      Use symbols everywhere for database configurations · ce9b197c
      eileencodes 提交于
      Previously in some places we used symbol keys, and in some places we used
      string keys. That made it pretty confusing to figure out in a particular
      place what type of configuration object you were working with.
      
      Now internally, all configuration hashes are keyed by symbols and
      converted to such on the way in.
      
      A few exceptions:
      
      - `DatabaseConfigurations#to_h` still returns strings for backward compatibility
      - Same for `legacy_hash`
      - `default_hash` previously could return strings, but the associated
        comment mentions it returns symbol-key `Hash` and now it always does
      
      Because this is a change in behavior, a few method renames have happened:
      
      - `DatabaseConfig#config` is now `DatabaseConfig#configuration_hash` and returns a symbol-key `Hash`
      - `ConnectionSpecification#config` is now `ConnectionSpecification#underlying_configuration_hash` and returns the `Hash` of the underlying `DatabaseConfig`
      - `DatabaseConfig#config` was added back, returns `String`-keys for backward compatibility, and is deprecated in favor of the new `configuration_hash`
      Co-authored-by: Neileencodes <eileencodes@gmail.com>
      ce9b197c
  20. 29 8月, 2019 1 次提交
    • E
      Call `while_preventing_writes` from `connected_to` · 66bc2ff6
      eileencodes 提交于
      If a user is using the middleware for swapping database connections and
      manually calling `connected_to` in a controller/model/etc without
      calling `while_preventing_writes(false)` there is potential for a race
      condition where writes will be blocked.
      
      While the user could _just_ call `while_preventing_writes` in the same
      place they call `connected_to` this would mean that all cases need to
      call two methods.
      
      This PR changes `connected_to` to call `while_preventing_writes`
      directly. By default we'll assume you don't want to prevent writes, but
      if called with `connected_to(role: :writing, prevent_writes: true)` or
      from the middleware (which calls `connected_to` this way) the writes
      will be blocked.
      
      For replicas, apps should use readonly users to enforce not writing
      rather than `while_preventing_writes` directly.
      
      Should fix the remaining issues in
      https://github.com/rails/rails/issues/36830
      66bc2ff6
  21. 14 8月, 2019 1 次提交
    • E
      Fix setting connection_specification_name on ActiveRecord::Base · 92d31b1f
      Eugene Kenny 提交于
      Since 2f8b3972, `ActiveRecord::Base` and
      `ApplicationRecord` use the same default `connection_specification_name`
      so that database connections configured on `ApplicationRecord` also
      apply to `ActiveRecord::Base`.
      
      However, when resolving the connection for `ApplicationRecord` we should
      continue to fall back to `ActiveRecord::Base` when there's no connection
      explicitly configured, so that setting `connection_specification_name`
      on `ActiveRecord::Base` affects `ApplicationRecord` and its descendants
      in this case as it did in previous versions.
      92d31b1f
  22. 13 6月, 2019 1 次提交
  23. 05 6月, 2019 1 次提交
    • E
      Treat ActiveRecord::Base and ApplicationRecord as "primary" · 2f8b3972
      eileencodes 提交于
      When someone has a multi-db application their `ApplicationRecord` will
      look like:
      
      ```ruby
      class ApplicationRecord < ActiveRecord::Base
        self.abstract_class = true
      
        connects_to database: { writing: :primary, reading: :replica }
      end
      ```
      
      This will cause us to open 2 connections to ActiveRecord::Base's
      database when we actually only want 1. This is because Rails sees
      `ApplicationRecord` and thinks it's a new connection, not the existing
      `ActiveRecord::Base` connection because the
      `connection_specification_name` is different.
      
      This PR changes `ApplicationRecord` classes to consider themselves the
      same as the "primary" connection.
      
      Fixes #36382
      2f8b3972
  24. 16 4月, 2019 1 次提交
  25. 09 4月, 2019 1 次提交
    • E
      Ensure a handler is set when using `connected_to` · b7cc1eb6
      eileencodes 提交于
      After looking at #35800 there is definitely an issue in the
      `connected_to` method although it's generally behaving. Here are the
      details:
      
      1) I added a default connection role - writing - to the connection
      handler lookup. I did this because otherwise if you did this:
      
      ```
      connected_to(databse: :development)
      ```
      
      And development wasn't a pre-established role it would create a new
      handler and connect using that. I don't think this is right so I've
      updated it to pick up the default (:writing) unless otherwise specified.
      To set a handler when using the database version pass a hash like you
      would to `connects_to`:
      
      ```
      connected_to(database: { readonly_slow: :development })
      ```
      
      This will connect the `development` database to the `readonly_slow`
      handler/role.
      
      2) I updated the tests to match this behavior that we expect.
      
      3) I updated the documentation to clarify that using `connected_to` with
      a `database` key will establish a new connection every time. This is
      exactly how `establish_connection` behaves and I feel this is correct.
      If you want to only establish a connection once you should do that in
      the model with `connects_to` and then swap on the role instead of on the
      database hash/key.
      
      4) In regards to #35800 this fixes the case where you pass a symbol to
      the db and not a hash. But it doesn't fix a case where you may pass an
      unknown handler to an abstract class that's not connected. This is
      tricky because technical AbstractFoo doesn't have any connections except
      for through ApplicationRecord, so in the case of the application that
      was shared we should only be swapping connections on ActiveRecord::Base
      because there are no other actual connections - AbstractFoo isn't needed
      since it's not establishing a new connection. If we need AbstractFoo to
      connect to a new handler we should establish that connection with the
      handler in AbstractFoo before trying to shard there.
      b7cc1eb6
  26. 02 2月, 2019 1 次提交
  27. 25 1月, 2019 1 次提交
    • E
      Fix error raised when handler doesn't exist · 1284f826
      Eileen Uchitelle 提交于
      While working on another feature for multiple databases (auto-switching)
      I observed that in development the first request won't autoload the
      application record connection for the primary database and may not yet
      know about the replica connection.
      
      In my test application this caused the application to thrown an error if
      I tried to send the first request to the replica before the replica was
      connected. This wouldn't be an issue in production because the
      application is preloaded.
      
      In order to fix this I decided to leave the original error message and
      delete the new error message. I updated the original error message to
      include the `role` to make it a bit clearer that the connection isn't
      established for that particular role.
      
      The error now reads:
      
      ```
      No connection pool with 'primary' found for the 'reading' role.
      ```
      
      A single database application will continue uisng the original error
      message:
      
      ```
      No connection pool with 'primary' found.
      ```
      1284f826
  28. 22 12月, 2018 1 次提交
    • E
      Raise helpful error when role doesn't exist · 22a12658
      Eileen Uchitelle 提交于
      If you try to call `connected_to` with a role that doesn't have an
      established connection you used to get an error that said:
      
      ```
      >> ActiveRecord::Base.connected_to(role: :i_dont_exist) { Home.first }
      
      ActiveRecord::ConnectionNotEstablished Exception: No connection pool
      with 'primary' found.
      ```
      
      This is confusing because the connection could be established but we
      spelled the role wrong.
      
      I've changed this to raise if the `role` used in `connected_to` doesn't
      have an associated handler. Users who encounter this should either check
      that the role is spelled correctly (writin -> writing), establish a
      connection to that role in the model with connects_to, or use the
      `database` keyword for the `role`.
      
      I think this will provide a less confusing error message for those
      starting out with multiple databases.
      22a12658
  29. 12 12月, 2018 1 次提交
  30. 15 11月, 2018 1 次提交
    • B
      Exercise `connected_to` and `connects_to` methods · cf01da28
      bogdanvlviv 提交于
      Since both methods are public API I think it makes sense to add these tests
      in order to prevent any regression in the behavior of those methods after the 6.0 release.
      
      Exercise `connected_to`
        - Ensure that the method raises with both `database` and `role` arguments
        - Ensure that the method raises without `database` and `role`
      
      Exercise `connects_to`
        - Ensure that the method returns an array of established connections(as mentioned
          in the docs of the method)
      
      Related to #34052
      cf01da28
  31. 31 10月, 2018 1 次提交
  32. 27 10月, 2018 1 次提交
  33. 11 10月, 2018 1 次提交
    • E
      Basic API for connection switching · 31021a8c
      Eileen Uchitelle 提交于
      This PR adds the ability to 1) connect to multiple databases in a model,
      and 2) switch between those connections using a block.
      
      To connect a model to a set of databases for writing and reading use
      the following API. This API supercedes `establish_connection`. The
      `writing` and `reading` keys represent handler / role names and
      `animals` and `animals_replica` represents the database key to look up
      the configuration hash from.
      
      ```
      class AnimalsBase < ApplicationRecord
        connects_to database: { writing: :animals, reading: :animals_replica }
      end
      ```
      
      Inside the application - outside the model declaration - we can switch
      connections with a block call to `connected_to`.
      
      If we want to connect to a db that isn't default (ie readonly_slow) we
      can connect like this:
      
      Outside the model we may want to connect to a new database (one that is
      not in the default writing/reading set) - for example a slow replica for
      making slow queries. To do this we have the `connected_to` method that
      takes a `database` hash that matches the signature of `connects_to`. The
      `connected_to` method also takes a block.
      
      ```
      AcitveRecord::Base.connected_to(database: { slow_readonly: :primary_replica_slow }) do
        ModelInPrimary.do_something_thats_slow
      end
      ```
      
      For models that are already loaded and connections that are already
      connected, `connected_to` doesn't need to pass in a `database` because
      you may want to run queries against multiple databases using a specific
      role/handler.
      
      In this case `connected_to` can take a `role` and use that to swap on
      the connection passed. This simplies queries - and matches how we do it
      in GitHub. Once you're connected to the database you don't need to
      re-connect, we assume the connection is in the pool and simply pass the
      handler we'd like to swap on.
      
      ```
      ActiveRecord::Base.connected_to(role: :reading) do
        Dog.read_something_from_dog
        ModelInPrimary.do_something_from_model_in_primary
      end
      ```
      31021a8c