Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

backup,restore: support backing up / restore system databases #1048

Merged
merged 17 commits into from
Apr 30, 2021

Conversation

YuJuncen
Copy link
Collaborator

What problem does this PR solve?

#872 and #679.

But this doesn't allow restore stats -- because the table IDs of the stats backed up are old table IDs, we need more efforts to rewrite them to make them take effect after the restoration.

What is changed and how it works?

This PR basically implemented #679 (comment), with some difference:

  • When the target table not exists, RENAME TABLE temp.xxx TO mysql.xxx would be used, for effectively restoring user tables created in mysql schema.
  • The user interface is the -f (the table filter) flag. Users must specify each table they want to restore by the table filter syntax (e.g. br restore full -f '*.*' -f '!mysql.*' -f 'mysql.usertable' to restore mysql.usertable).

Check List

Tests

  • Integration test

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
    • The document points out that table filter would always ignore system tables, that isn't real after this merged.
    • The usage of this feature.

Release Note

  • BR now support backing up user tables created in the mysql schema.

@ti-chi-bot ti-chi-bot added size/XL and removed size/L labels Apr 22, 2021
@YuJuncen
Copy link
Collaborator Author

/run-integration-tests

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

rest LGTM

tests/br_systables/run.sh Outdated Show resolved Hide resolved
tests/br_systables/run.sh Outdated Show resolved Hide resolved
errors.toml Outdated Show resolved Hide resolved
pkg/errors/errors.go Outdated Show resolved Hide resolved
pkg/restore/systable_restore.go Outdated Show resolved Hide resolved
pkg/errors/errors.go Outdated Show resolved Hide resolved
@YuJuncen YuJuncen requested a review from kennytm April 28, 2021 01:55
@YuJuncen
Copy link
Collaborator Author

YuJuncen commented Apr 28, 2021

ERROR 2003 (HY000): Can't connect to MySQL server on '127.0.0.1' (111) 🤔

listen tcp 0.0.0.0:10080: bind: address already in use

/run-integration-tests

@SunRunAway
Copy link
Contributor

Will global variables can be restored after this PR?

Not tested, but in theory they could.

Should you add this into the objective and and let the testing cover it?

@SunRunAway
Copy link
Contributor

Will global variables can be restored after this PR?

Not tested, but in theory they could.

@IANTHEREAL Please make it clear to customers.

@YuJuncen
Copy link
Collaborator Author

Will global variables can be restored after this PR?

Not tested, but in theory they could.

Should you add this into the objective and and let the testing cover it?

Done, a single value test shows that GLOBAL_VARIABLES can be restored. However, note this PR is a workaround for backup / restoring user tables created in mysql. I think it would be better to not promise any ability of backup / restoring system tables.

@3pointer
Copy link
Collaborator

/lgtm

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • 3pointer
  • kennytm

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 LGTM2 and removed status/LGT1 LGTM1 labels Apr 30, 2021
@YuJuncen
Copy link
Collaborator Author

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 04e0887

@SunRunAway
Copy link
Contributor

Will global variables can be restored after this PR?

Not tested, but in theory they could.

Should you add this into the objective and and let the testing cover it?

Done, a single value test shows that GLOBAL_VARIABLES can be restored. However, note this PR is a workaround for backup / restoring user tables created in mysql. I think it would be better to not promise any ability of backup / restoring system tables.

We should make it clear. Because once you released it, users will rely on this feature.

@ti-chi-bot ti-chi-bot merged commit b1ed365 into pingcap:master Apr 30, 2021
ti-srebot pushed a commit to ti-srebot/br that referenced this pull request Apr 30, 2021
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #1077

ti-srebot pushed a commit to ti-srebot/br that referenced this pull request Apr 30, 2021
@ti-srebot
Copy link
Contributor

cherry pick to release-5.0 in PR #1078

@YuJuncen
Copy link
Collaborator Author

@SunRunAway I added a warning over restoring system tables in the document.

cc @3pointer Please check whether it is necessary.

@IANTHEREAL
Copy link
Collaborator

If the system table cannot be restored correctly (referring to the system table similar to mysql.tidb, not the user data table under the mysql database), then there is no need to provide recovery methods to avoid misuse by users and affect their business

@SunRunAway
Copy link
Contributor

SunRunAway commented May 10, 2021

If the system table cannot be restored correctly, why you backup them, why you implemented this feature by this pull request?

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

Successfully merging this pull request may close these issues.

7 participants