| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The connection handler was using the RuntimeRegistry which kind of
implies it's a per thread registry. But it's actually per fiber.
If you have an application that uses fibers and you're using multiple
databases, when you switch the connection handler to swap connections
new fibers running on the same thread used to get a different connection
id. This PR changes the code to actually use a thread so that we get
the same connection.
Fixes https://github.com/rails/rails/issues/30047
[Eileen M. Uchitelle, Aaron Patterson, & Arthur Neeves]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
| |
connection
Related to #34052
|
|
|
|
|
| |
Add support for hash and url configs in database hash
of `ActiveRecord::Base.connected_to`.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
```
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since #33968 we stringify keys of database configuration
This commit adds more assertions in order to ensure that and prevent any
regression in the future.
Currently, if remove `to_s` added in #33968 from `env_name.to_s` on the
line
(activerecord/lib/active_record/database_configurations.rb:107), there is
no test that would fail. One of the added assertions should emphasize why we need
this `to_s`.
Follow up #33968
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
While the three-tier config makes it easier to define databases for
multiple database applications, it quickly became clear to offer full
support for multiple databases we need to change the way the connections
hash was handled.
A three-tier config means that when Rails needed to choose a default
configuration (in the case a user doesn't ask for a specific
configuration) it wasn't clear to Rails which the default was. I
[bandaid fixed this so the rake tasks could work](#32271) but that fix
wasn't correct because it actually doubled up the configuration hashes.
Instead of attemping to manipulate the hashes @tenderlove and I decided
that it made more sense if we converted the hashes to objects so we can
easily ask those object questions. In a three tier config like this:
```
development:
primary:
database: "my_primary_db"
animals:
database; "my_animals_db"
```
We end up with an object like this:
```
@configurations=[
#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbded10
@env_name="development",@spec_name="primary",
@config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>,
#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbdea90
@env_name="development",@spec_name="animals",
@config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>
]>
```
The configurations setter takes the database configuration set by your
application and turns them into an
`ActiveRecord::DatabaseConfigurations` object that has one getter -
`@configurations` which is an array of all the database objects.
The configurations getter returns this object by default since it acts
like a hash in most of the cases we need. For example if you need to
access the default `development` database we can simply request it as we
did before:
```
ActiveRecord::Base.configurations["development"]
```
This will return primary development database configuration hash:
```
{ "database" => "my_primary_db" }
```
Internally all of Active Record has been converted to use the new
objects. I've built this to be backwards compatible but allow for
accessing the hash if needed for a deprecation period. To get the
original hash instead of the object you can either add `to_h` on the
configurations call or pass `legacy: true` to `configurations.
```
ActiveRecord::Base.configurations.to_h
=> { "development => { "database" => "my_primary_db" } }
ActiveRecord::Base.configurations(legacy: true)
=> { "development => { "database" => "my_primary_db" } }
```
The new configurations object allows us to iterate over the Active
Record configurations without losing the known environment or
specification name for that configuration. You can also select all the
configs for an env or env and spec. With this we can always ask
any object what environment it belongs to:
```
db_configs = ActiveRecord::Base.configurations.configurations_for("development")
=> #<ActiveRecord::DatabaseConfigurations:0x00007fd1acbdf800
@configurations=[
#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbded10
@env_name="development",@spec_name="primary",
@config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>,
#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbdea90
@env_name="development",@spec_name="animals",
@config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>
]>
db_config.env_name
=> "development"
db_config.spec_name
=> "primary"
db_config.config
=> { "adapter"=>"sqlite3", "database"=>"db/development.sqlite3" }
```
The configurations object is more flexible than the configurations hash
and will allow us to build on top of the connection management in order
to add support for primary/replica connections, sharding, and
constructing queries for associations that live in multiple databases.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
At the moment these two ActiveRecord tests pass with `rake test:sqlite3`,
but fail with `ARCONN=sqlite3 bin/test`.
`Rails.root` is defined when running `bin/test`, but not when running
the rake task. When `Rails.root` is defined, `config[:database]` will
look something like `vagrant/rails/activerecord/db/primary.sqlite3`
instead of just `db/primary.sqlite3`.
(See https://github.com/rails/rails/blob/00caf95e14b90782ab17fbd6d2b930844df99980/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb#L27)
Relaxing `assert_equal` to `assert_match` will allow these tests to pass
regardless of how they are run.
I do have a question why we need both ways to run tests. I have been
using `bin/test` lately, but I see from #32426 that this is not the preferred
method.
|
|
|
|
|
|
| |
73e7aab behaved as expected on codeship, failing the build with
exactly these RuboCop violations. Hopefully `rubocop -a` will
have been enough to get a passing build!
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If you had a three-tier config, the `establish_connection` that's called
in the Railtie on load can't figure out how to access the default
configuration.
This is because Rails assumes that the config is the first value in the
hash and always associated with the key from the environment. With a
three tier config however we need to go one level deeper.
This commit includes 2 changes. 1) removes a line from `resolve_all`
which was parsing out the the environment from the config so instead of
getting
```
{
:development => {
:primary => {
:database => "whatever"
}
},
:animals => {
:database => "whatever-animals"
}
},
etc with test / prod
}
```
We'd instead end up with a config that had no attachment to it's
envioronment.
```
{
:primary => {
:database => "whatever"
}
:animals => {
:database => "whatever-animals"
}
etc - without test and prod
}
```
Not only did this mean that Active Record didn't know how to establish a
connection, it didn't have the other necessary configs along with it in
the configs list.
So fix this I removed the line that deletes these configs.
The second thing this commit changes is adding this line to
`establish_connection`
```
spec = spec[spec_name.to_sym] if spec[spec_name.to_sym]
```
When you have a three-tier config and don't pass any hash/symbol/env etc
to `establish_connection` the resolver will automatically return both
the primary and secondary (in this case animals db) configurations.
We'll get an `database configuration does not specify adapter` error
because AR will try to establish a connection on the `primary` key
rather than the `primary` key's config. It assumes that the
`development` or default env automatically will return a config hash,
but with a three-tier config we actually get a key and config `primary
=> config`.
This fix is a bit of a bandaid because it's not the "correct" way to
handle this situation, but it does solve our immediate problem. The new
code here is saying "if the config returned from the resolver (I know
it's called spec in here but we interchange our meanings a LOT and what
is returned is a three-tier config) has a key matching the "primary"
spec name, grab the config from the spec and pass that to the
estalbish_connection method".
This works because if we pass `:animals` or a hash, or `:primary` we'll
already have the correct configuration to connect with.
This fixes the case where we want Rail to connect with the default
connection.
Coming soon is a refactoring that should eliminate the need to do this
but I need this fix in order to write the multi-db rake tasks that I
promised in my RailsConf submission. `@tenderlove` and I are working on
the refactoring of the internals for connection management but it won't
be ready for a few weeks and this issue has been blocking progress.
|
| |
|
| |
|
|
|
|
|
|
| |
It doesn't have to do anything, but it shouldn't fail.
Fixes #31766.
|
|
|
|
|
|
| |
When removing a record from a has many through association, the counter
cache was being updated even if the through record halted the callback
chain and prevented itself from being destroyed.
|
|
|
|
|
| |
Commit 4ec5b0d was for fixing the regression #18787, but #28379 fixes
#18787 as well. So 4ec5b0d is no longer necessary.
|
|
|
|
| |
And prefer `assert_same` over `assert_equal`.
|
|
|
|
|
|
| |
Use whatever adapter-provided means we have available to ensure forked
children don't send quit/shutdown/goodbye messages to the server on
connections that belonged to their parent.
|
|\ |
|
| |\
| | |
| | | |
Make `type_map` to private because it is only used in the connection adapter
|
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
`type_map` is an internal API and it is only used in the connection
adapter. And also, some type map initializer methods requires passed
`type_map`, but those instances already has `type_map` in itself.
So we don't need explicit passing `type_map` to the initializers.
|
| |/ |
|
| |
| |
| |
| |
| | |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| | |
|
|/
|
|
|
| |
to address `Mysql2BooleanTest#test_column_type_without_emulated_booleans`
failure
|
|
|
|
|
|
| |
an empty string
Follow up of #27399.
|
|
|
|
| |
[Arthur Nogueira Neves & Matthew Draper]
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If a process is forked more than once, the pool was grabbing the oldest
spec, not the most recent spec. This wasn't noticed before because most
folks are lilely forking the process only once.
If you're forking the process multiple times however the wrong spec name
will be returned and an incorrect connection will be used for the
process.
This fixes the issue by reversing the list of spec names so we can grab
the most recent spec rather than the oldest spec.
|
| |
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
Style/SpaceBeforeBlockBraces
Style/SpaceInsideBlockBraces
Style/SpaceInsideHashLiteralBraces
Fix all violations in the repository.
|
| |
|
| |
|
|
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
| |
|
|
|
|
|
| |
Instead of passing a separete name variable, we can make the resolver
merge a name on the config, and use that before creating the Specification.
|
|
|
|
|
|
| |
When calling remove_connection in a subclass, that should not fallback
to the parent, otherwise it will remove the parent connection from the
handler.
|
|
|
|
|
|
|
|
| |
We cannot cache the connection_specification_name when it doesnt
exist. Thats because the parent value could change, and we should keep
failling back to the parent. If we cache that in a children as an ivar,
we would not fallback anymore in the next call, so the children would
not get the new parent spec_name.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
remove_connection
When calling `remove_connection` on a model, we delete the pool so we also
need to reset the `connection_specification_name` so it will fallback to
the parent.
This was the current behavior before rails 5, which will fallback to the
parent connection pool.
[fixes #24959]
Special thanks to @jrafanie for working with me on this fix.
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
ConnectionHandler will not have any knowlodge of AR models now, it will
only know about the specs.
Like that we can decouple the two, and allow the same model to use more
than one connection.
Historically, folks used to create abstract AR classes on the fly in
order to have multiple connections for the same model, and override the
connection methods.
With this, now we can override the `specificiation_id` method in the
model, to return a key, that will be used to find the connection_pool
from the handler.
|
|
|
|
| |
[ci skip]
|
| |
|