Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

restore: support restore from s3 #361

Merged
merged 27 commits into from
Sep 4, 2020
Merged

restore: support restore from s3 #361

merged 27 commits into from
Sep 4, 2020

Conversation

glorv
Copy link
Contributor

@glorv glorv commented Jul 30, 2020 •
edited
Loading

What problem does this PR solve?

Support restore data files from s3 storage
close #69

Related change in br: pingcap/br#446

What is changed and how it works?

Replace the direct file IO by ExternalStorage interface and share the storage implements from br.

Check List

Tests

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

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

@glorv glorv added the status/WIP Work in progress label Jul 30, 2020
@glorv glorv requested review from 3pointer and kennytm July 30, 2020 07:55
kennytm
kennytm reviewed Jul 30, 2020
View reviewed changes
go.mod Outdated Show resolved Hide resolved
@glorv glorv requested a review from kennytm July 30, 2020 09:38
@glorv glorv removed the status/WIP Work in progress label Aug 18, 2020
kennytm
kennytm reviewed Aug 18, 2020
View reviewed changes
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.

(let's block this until #366 is merged)

lightning/config/config.go Outdated
return errors.Annotatef(err, "covert data-source-dir '%s' to absolute path failed", cfg.Mydumper.SourceDir)
}
cfg.Mydumper.SourceDir = fmt.Sprintf("file://%s", absPath)
u, err = url.Parse(cfg.Mydumper.SourceDir)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
u, err = url.Parse(cfg.Mydumper.SourceDir)
u.Path = absPath
u.Scheme = "file"

@glorv glorv mentioned this pull request Aug 19, 2020
Merged
@glorv glorv requested a review from kennytm August 20, 2020 09:45
kennytm
kennytm reviewed Aug 28, 2020
View reviewed changes
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

lightning/mydump/reader_test.go Outdated Show resolved Hide resolved
lightning/mydump/reader.go Outdated Show resolved Hide resolved
lightning/mydump/loader.go Outdated Show resolved Hide resolved
@glorv
Copy link
Contributor Author

glorv commented Sep 3, 2020

/run-all-tests

@kennytm
Copy link
Collaborator

kennytm commented Sep 3, 2020

LGTM

@kennytm kennytm added the status/LGT1 One reviewer already commented LGTM (LGTM1) label Sep 3, 2020
@glorv
Copy link
Contributor Author

glorv commented Sep 4, 2020

/run-all-tests

2 similar comments
@glorv
Copy link
Contributor Author

glorv commented Sep 4, 2020

/run-all-tests

@glorv
Copy link
Contributor Author

glorv commented Sep 4, 2020

/run-all-tests

3pointer
3pointer approved these changes Sep 4, 2020
View reviewed changes
Copy link
Contributor

@3pointer 3pointer left a comment

Choose a reason for hiding this comment

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

LGTM

@kennytm kennytm merged commit d23cf02 into master Sep 4, 2020
@kennytm kennytm deleted the support-s3 branch September 4, 2020 07:57
@kennytm kennytm added the Should Update Docs Should update docs after this PR is merged. Remove this label once the docs are updated label Sep 4, 2020
@scsldb scsldb added this to the 4.0.10 milestone Oct 16, 2020
@scsldb scsldb added feature-request This issue is a feature request feature/accepted labels Oct 16, 2020
@kennytm kennytm mentioned this pull request Nov 5, 2020
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kennytm kennytm kennytm left review comments

@3pointer 3pointer 3pointer approved these changes

Assignees
No one assigned
Labels
feature/accepted feature-request This issue is a feature request Should Update Docs Should update docs after this PR is merged. Remove this label once the docs are updated status/LGT1 One reviewer already commented LGTM (LGTM1)
Projects
None yet
Milestone
v4.0.10
Development

Successfully merging this pull request may close these issues.

Restore from S3 compatible API?
4 participants
@glorv @kennytm @3pointer @scsldb