| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Performance cops will be extracted from RuboCop to RuboCop Performance
when next RuboCop 0.68 will be released.
https://github.com/rubocop-hq/rubocop/issues/5977
RuboCop 0.67 is its transition period.
Since rails/rails repository uses Performance cops, This PR added
rubocop-performance gem to Gemfile.
And this PR fixes some offenses using the following auto-correct.
```console
% bundle exec rubocop -a
Offenses:
activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:212:26:
C: [Corrected] Layout/SpaceAroundOperators: Operator =
> should be surrounded by a single space.
"primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" }
^^
activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:239:26:
C: [Corrected] Layout/SpaceAroundOperators: Operator => should be
surrounded by a single space.
"primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" }
^^
actionview/test/template/resolver_shared_tests.rb:1:1: C: [Corrected]
Style/FrozenStringLiteralComment: Missing magic comment #
frozen_string_literal: true.
module ResolverSharedTests
^
actionview/test/template/resolver_shared_tests.rb:10:33: C: [Corrected]
Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in
default value assignment.
def with_file(filename, source="File at #{filename}")
^
actionview/test/template/resolver_shared_tests.rb:106:5: C: [Corrected]
Rails/RefuteMethods: Prefer assert_not_same over refute_same.
refute_same a, b
^^^^^^^^^^^
2760 files inspected, 5 offenses detected, 5 offenses corrected
```
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When I wrote the `connected_to` and `connects_to` API's I wrote them
with the idea in mind that it didn't really matter what the
handlers/roles were called as long as those connecting to the roles knew
which one wrote and which one read.
With the introduction of the middleware Rails begins to assume it's
`writing` and `reading` and there's no room for other roles. At GitHub
we've been using this method for a long time so we have a ton of legacy
code that uses different handler names `default` and `readonly`. We
could rename all our code but I think this is better for a few reasons:
- Legacy apps that have been using multiple databases for a long time
can have an eaiser time switching.
- If we later find this to cause more issues than it's worth we can
easily deprecate.
- We won't force old apps to rewrite the resolver middleware just to use
a different handler.
Adding the writing_role/reading_role required that I move the code that
creates the first handler for writing to the railtie. If I didn't move
this the core class would assign the handler before I was able to assign
a new one in my configuration and I'd end up with 3 handlers instead of
2.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
```
|
| |
|
|
| |
Since the `current_role` is public API.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
| |
This can be used to check the currently connected role. It's meant to
mirror AR::Base.connected_to
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
```
|
