Part 3: Multi-db Improvements, identifying replica configurations#33770
Part 3: Multi-db Improvements, identifying replica configurations#33770eileencodes merged 3 commits intorails:masterfrom
Conversation
This allows the user to add `replica: true` to the database config to signify the connection should be treated as readonly. This will be useful so we can ignore structure dumps or migrations (or creating / deleting etc) the readonly connection for the databases. These are paired with a write database which is where the create/drop/migrate should be run. This allows us to ask the connection if it's for a replica readonly db or a primary write db.
Checks if the config has a "replica" key, if so the configuration is for a replica database. This is useful for excluding replicas from the configurations list when creating the rake tasks or running rake tasks. For example, we don't want to create the primary and primary_readonly. They're the same database.
Changes the `configs_for` method from using traditional arguments to using kwargs. This is so I can add the `include_replicas` kwarg without having to always include `env_name` and `spec_name` in the method call. `include_replicas` defaults to false because everywhere internally in Rails we don't want replicas. `configs_for` is for iterating over configurations to create / run rake tasks, so we really don't ever need replicas in that case.
5a59643 to
a572d22
Compare
| end | ||
|
|
||
| def replica? | ||
| @config[:replica] || false |
There was a problem hiding this comment.
@eileencodes Is it in the scope of this PR for each database adapter to verify if the replica database is indeed a replica to help catch user errors? We have this guard in place at Discourse whenever we switch over to a replica database:
value = connection.exec_query("SELECT pg_is_in_recovery()").rows[0][0]
raise "Replica database server is not in recovery mode." if !value
There was a problem hiding this comment.
At GitHub we have rw databases (primary) and ro databases (replica). The replicas have different users than the primary ones. In addition we also have readonly connections - a rw database can be connected to in a ro way. This change is for the first part - to be able to identify the replica database in the configurations list. Later when I implement the connection switching I'll handle your case as well since we need that at GitHub, but that's going to be a separate PR.
There was a problem hiding this comment.
That is great to hear :) Thank you for sharing.
|
@eileencodes Awesome! |
… the CHANGELOG Since rails#33770 `#configs_for` changed method signature and it isn't supposed to work with a passed block.
I'm going to start opening smaller PR's now that the refactoring is merged. This PR adds a few things:
replicaoption to the configurationsHashConfigandUrlConfigforreplica?that checks forconfig["replica"]keyconfigs_forto take kwargsconfigs_forto take a kwarg forinclude_replicasthat defaults to false. When you're creating/dropping/migrating dbs you don't need to run the create/drop/migrate for the replicas as well. I defaulted to not including replicas because 99% of the time when you're iterating over the dbs (like creating the tasks, running the tasks etc) you don't actually want the replicas. This prevents messages like "database already exists" when running the create command.cc/ @tenderlove @matthewd @rafaelfranca