-
Notifications
You must be signed in to change notification settings - Fork 72
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
always use pg_dump for DB backups #893
Conversation
for_feature :candlepin_database | ||
preparation_steps { Checks::Candlepin::DBUp.new unless feature(:candlepin_database).local? } | ||
param :backup_dir, 'Directory where to backup to', :required => true | ||
param :tar_volume_size, 'Size of tar volume (indicates splitting)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's funny this was defined, but never passed to the procedure (heck, even the CLI param is called --split-pulp-tar
), so yay less unused complexity?
Gotta find out how to move that check to the prep steps. |
7f98178
to
3f8dfb4
Compare
@evgeni Are you holding on merge to perform testing first? |
Yeah, we were trying to run tests and they ended up red, so … waiting |
when restoring incremental backups, you can run in the situation where the on-disk representation of the database changes between the restore of the base and the increment, leading to a broken database. by using logical DB dumps, we ensure that the restore always work at the cost that increments are slightly bigger as they now always include a full DB dump this also allows simplification of the code as we now only have one type of DB backup to support
ah, nice. aaac56d did actually break things for this PR \o/ |
theforeman/foreman_maintain#893 switched the db backups to dump
theforeman/foreman_maintain#893 switched the db backups to dump
includes #891 for the checks