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

backup: optimize backupmeta to reduce memory useage. #1171

Merged
merged 45 commits into from
Jun 10, 2021

Conversation

3pointer
Copy link
Collaborator

@3pointer 3pointer commented Jun 2, 2021

What problem does this PR solve?

solve #819

What is changed and how it works?

  1. Design new version of backupmeta(backupmeta v2).
  2. Split the ddls,schemas,files to new metafile.
  3. Add a hidden flag use-backupmeta-v2 for compatibility.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Release note

  • Change the structure of backupmeta to reduce memory usage in Backup and Restore.

@glorv
Copy link
Collaborator

glorv commented Jun 9, 2021

/run-all-tests

Copy link
Collaborator

@YuJuncen YuJuncen left a comment

Choose a reason for hiding this comment

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

(the read part) rest lgtm

pkg/metautil/metafile.go Show resolved Hide resolved
@3pointer
Copy link
Collaborator Author

3pointer commented Jun 9, 2021

/run-integration-tests

1 similar comment
@3pointer
Copy link
Collaborator Author

3pointer commented Jun 9, 2021

/run-integration-tests

@3pointer
Copy link
Collaborator Author

3pointer commented Jun 9, 2021

/run-integration-tests

1 similar comment
@3pointer
Copy link
Collaborator Author

3pointer commented Jun 9, 2021

/run-integration-tests

@ti-chi-bot ti-chi-bot added the status/LGT1 LGTM1 label Jun 9, 2021
@@ -65,6 +67,7 @@ type BackupConfig struct {
GCTTL int64 `json:"gc-ttl" toml:"gc-ttl"`
RemoveSchedulers bool `json:"remove-schedulers" toml:"remove-schedulers"`
IgnoreStats bool `json:"ignore-stats" toml:"ignore-stats"`
UseBackupMetaV2 bool `json:"use-backupmeta-v2"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should better be a flag accepting an integer like --backupmeta-version=2, in case we need a v3 in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, that sounds better, but it's a hidden flag, let's change it in meta v3.

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • glorv
  • 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 submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added status/LGT2 LGTM2 and removed status/LGT1 LGTM1 labels Jun 9, 2021
@glorv
Copy link
Collaborator

glorv commented Jun 10, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: a3ab71d

@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #1193.

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.

6 participants