Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the new schema_migrations_ran table to track remote schema migrations #18393

Merged
merged 3 commits into from
Jan 29, 2019

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Jan 24, 2019

If a column is added to MiqRegion, that causes replication to stop. With replication stopped we sit in the loop forever waiting for the MiqRegion record for the remote region to be updated with the migrations_ran. If we instead use the new dedicated table schema_migrations_ran, we no longer need to worry about model tables adding or removing columns and breaking replication and upgrades.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1668800
This should block the release. Without it, users with replication enabled will not be able to migrate their global region through "20180920085721 AddMaintenanceZoneIdToRegion".

@bdunne bdunne requested a review from carbonin January 24, 2019 21:35
@bdunne bdunne added the blocker label Jan 24, 2019
@bdunne bdunne changed the title Check the remote schema_migrations table if the migration is not listed [WIP] Check the remote schema_migrations table if the migration is not listed Jan 24, 2019
@miq-bot miq-bot added the wip label Jan 24, 2019
@bdunne bdunne changed the title [WIP] Check the remote schema_migrations table if the migration is not listed Check the remote schema_migrations table if the migration is not listed Jan 25, 2019
@miq-bot miq-bot removed the wip label Jan 25, 2019
@bdunne bdunne force-pushed the migration_bz_1668800 branch 3 times, most recently from 727a053 to ade3056 Compare January 25, 2019 19:21
lib/extensions/ar_migration.rb Outdated Show resolved Hide resolved
lib/extensions/ar_migration.rb Outdated Show resolved Hide resolved
lib/extensions/ar_migration.rb Outdated Show resolved Hide resolved
lib/extensions/ar_migration.rb Show resolved Hide resolved
lib/extensions/ar_migration.rb Outdated Show resolved Hide resolved
Copy link
Member

@carbonin carbonin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment otherwise it looks good. Still going through some test cases though.

lib/extensions/ar_migration.rb Show resolved Hide resolved
@carbonin
Copy link
Member

@bdunne probably good to fix up whatever specs are failing. I don't see the general form of this changing much at this point

spec/lib/extensions/ar_migration_spec.rb Outdated Show resolved Hide resolved
spec/lib/extensions/ar_migration_spec.rb Outdated Show resolved Hide resolved
@carbonin
Copy link
Member

@bdunne can you also update the PR title and description to reflect the new direction we went with this?

@bdunne bdunne changed the title Check the remote schema_migrations table if the migration is not listed Use the new schema_migrations_ran table to track remote schema migrations Jan 29, 2019
@miq-bot
Copy link
Member

miq-bot commented Jan 29, 2019

Some comments on commits bdunne/manageiq@a80c633~...8fd91be

spec/lib/extensions/ar_migration_spec.rb

  • ⚠️ - 55 - Detected puts. Remove all debugging statements.

@miq-bot
Copy link
Member

miq-bot commented Jan 29, 2019

Checked commits bdunne/manageiq@a80c633~...8fd91be with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 4 offenses detected

lib/extensions/ar_migration.rb

spec/lib/extensions/ar_migration_spec.rb

@carbonin carbonin merged commit 8cca781 into ManageIQ:master Jan 29, 2019
@carbonin carbonin added this to the Sprint 104 Ending Feb 4, 2019 milestone Jan 29, 2019
@Fryguy Fryguy deleted the migration_bz_1668800 branch January 29, 2019 19:03
simaishi pushed a commit that referenced this pull request Jan 29, 2019
Use the new schema_migrations_ran table to track remote schema migrations

(cherry picked from commit 8cca781)

https://bugzilla.redhat.com/show_bug.cgi?id=1668800
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 85a1e4ec6f0e7f0c247e62a77cfee65e1d5bde45
Author: Nick Carboni <ncarboni@redhat.com>
Date:   Tue Jan 29 14:02:19 2019 -0500

    Merge pull request #18393 from bdunne/migration_bz_1668800
    
    Use the new schema_migrations_ran table to track remote schema migrations
    
    (cherry picked from commit 8cca7810496cba4871de85248157494fec93e0a4)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1668800

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants