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

Already on GitHub? Sign in to your account

Ticket 15490/myqlbackup improvements #84

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

grooverdan commented Jul 12, 2012

nice to reduce cpu and io.

second commit could be problematic. When bin logging isn't enabled it errors with:
mysqldump: Error: Binlogging on server not active

acceptable?

Member

hunner commented Aug 24, 2012

Hi @grooverdan, thanks for your pull request.

Could you provide a little description around the reason you're niceing the process? Also, could you perhaps add a conditional around the nice and the master-data argument such that they can be independently enabled, but not by default? Thanks!

I'm not very familiar with the mysqlbackup.sh script; is it generally useful in your environment?

Member

ryanycoleman commented Oct 13, 2012

@grooverdan, ping.

Contributor

grooverdan commented Oct 19, 2012

Could you provide a little description around the reason you're niceing the process?

To reduce the cpu and IO priority of the backup process so that other tasks, who's actions are likely immediately required, can continue with minimum latency.

Also, could you perhaps add a conditional around the nice and the master-data argument such that they can be independently enabled, but not by default? Thanks!

conditional nice: name a scenario where the backup is the most important task on the system and its CPU and IO priority should be higher. If there is a lower priority process it can always be given a higher nice value to give it less priority.

conditional master-data: this does need binlogs and a RELOAD priv for the backup privs. You've already specified at the top of templates/mysqlbackup.sh.erb that RELOAD is reuqired. If you try to do partial restores without binlogs your in a world of pain. If you also try to restore in a replicated environment without the master log position you also are in a world of pain.

Though it may seem friendly to keep these both off by default you are only condemning the future sysadmin for more pain when they try to perform a restoration. Please don't make these optional.

I'm not very familiar with the mysqlbackup.sh script; is it generally useful in your environment?

Its simple and gets the job done quick. 30 days expiry is a nice default. Every other mysql backupscript I've seen has always been a minor variant of this one.

Contributor

grooverdan commented Oct 23, 2012

also rational against making master-data=2 optional. The information is added as a comment only. It requires interaction on the database restorer's part to use this data. Making it optional will remove the availability of this information.

Member

hunner commented Nov 7, 2012

@grooverdan The reason I wanted to make these things conditional is for backwards-compatibility reasons. Any changes that are available via a parameter are available to use but won't surprise anyone with changes in behavior.

Contributor

grooverdan commented Nov 8, 2012

running at a lower priority is a backwards compatible change and is harmless.

master-data=2 is backwards compatible if binlogs are enabled. If the don't have binlogs enabled they don't have the ability to restore any data changes between the backup and the current time. I'd rather surprise people to say their backups aren't quite working than to surprise than saying they can't get a full restore.

I could do something that emits a warning if they don't have binlogs enabled and revert to the current mechanism? Is this acceptable?

The other thing I've found useful is to have a schema backup without data:
mysqldump --skip-lock-tables --no-data --quick -u${USER} -p${PASS} --all-databases > ...

Contributor

grooverdan commented Nov 8, 2012

i could even check for the existence of nice before running it.

Member

hunner commented Jan 11, 2013

Okay, I think this may be decent, but I'd like to run it by a few people first. Thanks for your patience!

CLA Signed by grooverdan on 2011-12-21 21:00:00 -0800

Contributor

apenney commented Jul 2, 2013

Hi,

@grooverdan Would you mind rebasing this against master if you're still interested in having this work with nice? Someone added backupcompress as a param along the way so it can't cleanly merge anymore.

Contributor

apenney commented Sep 23, 2013

Hi,

I've recently merged in a very large changeset that completely redesigns this module. As a result I'm afraid this PR is no longer mergeable and possibly no longer valid (sorry, mass message for all PRs) due to the changes to binding and the removal of most of the parameters for my.cnf (as it's handed by an override hash currently).

I'm going to close all existing PRs that can't be merged and ask you to take a look at rebasing them against master or redeveloping them against master if the feature is still important to you. I hope that many of the existing PRs won't be needed in the refactoring!

Thanks for your patience everyone, I know this is pain.

@apenney apenney closed this Sep 23, 2013

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