Make replication excludes specific by region #12592
Conversation
|
Leaving this as WIP until I can figure out if we need a migration for handling upgrades. The issue is that users already have replication excludes set in the settings and we would want to continue using those after the upgrade (especially if they are upgrading from rubyrep to pglogical). |
|
@Fryguy I think the easier way to go regarding upgrades is to just reset the excludes to the default. If we try to do something else we run into logical issues around what happens if different servers have different excludes at the time of upgrade. Because we only use them when pglogical is enabled we could have files with different contents on different servers and run into exactly the issue we are trying to solve. If users are already using pglogical replication this will be fine and will pick up their configured excludes from the pglogical tables. The only people this will effect are the ones going from rubyrep to pglogical that want to keep their old set of exclude tables, but we recommend that they use the default anyway. |
|
Another side-effect of this change is that excludes don't persist if you go from having pglogical configured (remote replication type) to not configured and back again. This is just because we are keeping track of the excludes by reading pglogical's tables. I don't think that is too surprising a behavior, but it is a change from the previous way we did it. |
This will allow us to move away from storing this database centric setting as related to the server. https://bugzilla.redhat.com/show_bug.cgi?id=1389821
This also exposes a new method, `#active_excludes`, to retrieve the current excludes being used by pglogical. This will pull the list of tables directly from the pglogical tables in the database rather than using an intermediate list of settings related to the server. https://bugzilla.redhat.com/show_bug.cgi?id=1389821
We can't use the instance methods with the queue because this is not an ActiveRecord object. https://bugzilla.redhat.com/show_bug.cgi?id=1389821
This is no longer needed because the exclude tables are not stored in the global Settings anymore. https://bugzilla.redhat.com/show_bug.cgi?id=1389821
…ances This means that `MiqPglogical.new.refresh_excludes` will not change the excluded tables back to the default, but will leave them as they currently are. https://bugzilla.redhat.com/show_bug.cgi?id=1389821
carbonin
force-pushed
the
make_replication_excludes_specific_by_region
branch
from
4f86504
to
aefb554
Compare
carbonin
changed the title
Totally forgot about that, and with that I agree with everything else. |
| :value => %w(schema_migrations)) | ||
| server_two.settings_changes | ||
| .create!(:key => described_class::EXCLUDES_KEY, | ||
| :value => %w(ar_internal_metadata)) |
There was a problem hiding this comment.
Minor but you might want one more test of some other change to ensure it's not being killed
There was a problem hiding this comment.
You can just set resource_type and resource_id to any arbitrary value instead
| describe RemoveReplicationExcludesFromSettings do | ||
| let(:settings_change_stub) { migration_stub(:SettingsChange) } | ||
| let(:server_one) { FactoryGirl.create(:miq_server) } | ||
| let(:server_two) { FactoryGirl.create(:miq_server) } |
There was a problem hiding this comment.
Avoid using factories in migration specs as they load the real models.
|
A couple of minor comments with the specs, but otherwise this looks good! |
Because we have removed the excludes from the settings.yml file we should remove the changes made in the database. This will leave current installations of pglogical in the same state they were in (we will read the excludes from the pglogical tables), but will reset new installations created when moving from rubyrep to pglogical. https://bugzilla.redhat.com/show_bug.cgi?id=1389821
carbonin
force-pushed
the
make_replication_excludes_specific_by_region
branch
from
aefb554
to
0e79109
Compare
|
Checked commits carbonin/manageiq@f50c798~...0e79109 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 lib/miq_pglogical.rb
|
|
@chessbyte I just realized this has a SQL migration, so it can't be backported directly. @carbonin Can you do a separate backport PR without the SQL migration? |
|
Euwe backport PR #12625 |
|
Backported to Euwe via #12625 |
chessbyte
modified the milestones:
Sprint 50 Ending Dec 5, 2016,
Sprint 49 Ending Nov 14, 2016
This PR will move the replication exclude settings out of the global settings and instead read and write directly to the pglogical configuration in the database. This change will allow us to decouple the replication settings from the server record and have them be consistent across all the servers which talk to the same database.
Previously, each server could set the exclude tables independently which could cause workers on different servers to add and remove tables rapidly as they updated their configuration.
A separate file will be kept to determine the default replication excludes when a node is created.
This also adds an interface for the UI to display the current excludes and refresh them using the queue.
https://bugzilla.redhat.com/show_bug.cgi?id=1389821
@Fryguy @gtanzillo please review
/cc @lgalis