Make db restore more reliable #16942
Conversation
|
Also marking this as WIP until I can work out specs and the correct merge ordering for this change and the other PRs related to this one. |
| sleep 5 | ||
| end | ||
|
|
||
| connection_count = backup_type == :basebackup ? VmdbDatabaseConnection.unscoped.count : VmdbDatabaseConnection.count |
| end | ||
|
|
||
| private_class_method def self.application_connections? | ||
| VmdbDatabaseConnection.all.map(&:application_name).any? { |app_name| app_name.start_with?("MIQ") } |
There was a problem hiding this comment.
Should unscoped be used here too?
There was a problem hiding this comment.
No, because all it does when used with .all is remove the database restriction, but since this one is for application connections we can only query against the configured database.
| 60.times do | ||
| break if VmdbDatabaseConnection.where("application_name LIKE 'pglogical manager%'").count.zero? | ||
| _log.info("Waiting for pglogical connections to close...") | ||
| sleep 5 |
lib/evm_database_ops.rb
Outdated
| _log.info("[#{db_opts[:dbname]}] database has been restored from file: [#{uri}]") | ||
| uri | ||
| end | ||
|
|
||
| private_class_method def self.without_connections | ||
| params = ActiveRecord::Base.remove_connection |
There was a problem hiding this comment.
do we want to call ActiveRecord::Base.connection_pool.disconnect! or somethign like that to disconnect all connections in the pool?
There was a problem hiding this comment.
Probably. Is that the better way to do this?
There was a problem hiding this comment.
@carbonin I think so. If other threads have connections, I don't believe remove_connection will disconnect those connections. I believe connection_pool.disconnect! is more what we want.
|
I like the direction that this is going. What remains to be done? |
carbonin
force-pushed
the
make_db_restore_more_reliable
branch
from
36d3033
to
c0fcb12
Compare
|
I'll unwip this one. I think we can merge this first, then we can merge the others and release |
carbonin
changed the title
carbonin
force-pushed
the
make_db_restore_more_reliable
branch
from
c0fcb12
to
3c9101e
Compare
This allows us to do more of the processing using existing code rather than the current implementation which does the checks in different places/repos and does a lot of shelling out to psql rather than using a real database connection. This also makes the error cases more explicit which fixes an issue where a user could not restore a pg_dump backup when pglogical was running. https://bugzilla.redhat.com/show_bug.cgi?id=1540686
We need the rails environment to access the VmdbDatabaseConnection model
To do this, we need to disconnect the connection we created when requiring the :environment rake task to run this method.
carbonin
force-pushed
the
make_db_restore_more_reliable
branch
from
0740031
to
1876da3
Compare
|
Checked commits carbonin/manageiq@898b63d~...1876da3 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 |
Make db restore more reliable (cherry picked from commit 8d24b56) https://bugzilla.redhat.com/show_bug.cgi?id=1553903
|
Gaprindashvili backport details: |
This PR moves much of the logic around how what to check for when we do a database restore into
EvmDatabaseOpsrather thanPostgresAdminThis allows us to use the models and native PG connections to do things like remove pglogical and check for connections to the database with particular application names.
This also brings all of the connection checking into one class (and repo) rather than doing some checking in the console, then some other checking in
PostgresAdminand also sometimes failing indb:dropThis change also fixes https://bugzilla.redhat.com/show_bug.cgi?id=1540686 which was caused by this confusing handling for different backup types.