Skip to content
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.

Sign up for GitHub

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

Jump to bottom

support backup to S3 with br #1280

Merged
merged 15 commits into from
Dec 30, 2019
Merged

support backup to S3 with br #1280

merged 15 commits into from
Dec 30, 2019

Conversation

DanielZhangQD
Copy link
Contributor

What problem does this PR solve?

backup to S3 with br for issue #1249

What is changed and how does it work?

Support new BR for backup to S3

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Code changes

  • Has Go code change
  • Has documents change

Side effects

Related changes

  • Need to update the documentation

Does this PR introduce a user-facing change?:

Support backup to S3 with BR

@DanielZhangQD DanielZhangQD mentioned this pull request Dec 4, 2019
Closed
4 tasks
@gregwebs
Copy link
Contributor

gregwebs commented Dec 4, 2019

I see this shelling out to run br but I expected br to be ran as a job. I thought br could take a very long time to complete.

@DanielZhangQD
Copy link
Contributor Author

I see this shelling out to run br but I expected br to be ran as a job. I thought br could take a very long time to complete.

This PR reuses the backup-manager introduced in the current backup feature with mydumper, it is run in a job and backup-manager will call br to backup data.

@DanielZhangQD
Copy link
Contributor Author

/run-e2e-in-kind

@DanielZhangQD DanielZhangQD mentioned this pull request Dec 4, 2019
Merged
@tennix tennix mentioned this pull request Dec 16, 2019
Open
9 tasks
@DanielZhangQD DanielZhangQD changed the title support backup to S3 with br [WIP]support backup to S3 with br Dec 17, 2019
@DanielZhangQD
Copy link
Contributor Author

DanielZhangQD commented Dec 17, 2019 •
edited
Loading

Code updated due to BR cli options change in pingcap/br#103.
TODO:

  1. Get commit TS from br backup meta.
  2. After BR should provide the total size of one backup br#95 addressed, update code to get backup size from br backup meta.

DanielZhangQD
DanielZhangQD commented Dec 19, 2019
View reviewed changes
cmd/backup-manager/app/backup/backup.go
@@ -14,200 +14,128 @@
package backup
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the backup file for BR.

cmd/backup-manager/app/backup/manager.go
@@ -14,43 +14,38 @@
package backup
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the backup manager for BR.

cmd/backup-manager/app/cmd/export.go
@@ -0,0 +1,79 @@
// Copyright 2019 PingCAP, Inc.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is renamed from cmd/backup-manager/app/cmd/backup.go.

cmd/backup-manager/app/export/export.go
@@ -0,0 +1,203 @@
// Copyright 2019 PingCAP, Inc.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is renamed from cmd/backup-manager/app/backup/backup.go.

cmd/backup-manager/app/export/manager.go
@@ -0,0 +1,239 @@
// Copyright 2019 PingCAP, Inc.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is renamed from cmd/backup-manager/app/backup/manager.go.

@DanielZhangQD DanielZhangQD changed the title [WIP]support backup to S3 with br support backup to S3 with br Dec 19, 2019
@DanielZhangQD
Copy link
Contributor Author

@tennix @onlymellb @shuijing198799 @gregwebs PTAL the new commits to line up with the latest BR update.

@DanielZhangQD
Copy link
Contributor Author

Update scheduled backup with BR, PTAL.

@DanielZhangQD
Copy link
Contributor Author

@onlymellb comments addressed, PTAL.

onlymellb
onlymellb approved these changes Dec 30, 2019
View reviewed changes
Copy link
Contributor

@onlymellb onlymellb left a comment

Choose a reason for hiding this comment

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

LGTM

shuijing198799
shuijing198799 approved these changes Dec 30, 2019
View reviewed changes
Copy link
Contributor

@shuijing198799 shuijing198799 left a comment

Choose a reason for hiding this comment

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

LGTM

@aylei aylei merged commit 4cec198 into pingcap:master Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregwebs gregwebs gregwebs left review comments

@tennix tennix tennix left review comments

@onlymellb onlymellb onlymellb approved these changes

@shuijing198799 shuijing198799 shuijing198799 approved these changes

@cofyc cofyc Awaiting requested review from cofyc

Assignees

@tennix tennix

@onlymellb onlymellb

Labels
area/backup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@DanielZhangQD @gregwebs @tennix @onlymellb @shuijing198799 @aylei