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.

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

external_storage: fix GCS download error, support GCS endpoints, and refactoring (#7734) #7962

Closed
wants to merge 6 commits into from

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented May 29, 2020

cherry-pick #7734 to release-3.1


What problem does this PR solve?

Issue Number: close #7625

Problem Summary:

It was found that the GCS external_storage can only download 8 KB of data. This PR fixes the issue.

Additionally, tame_gcs hard-coded the request URLs, making the custom endpoint ineffective (EmbarkStudios/tame-gcs#21). This PR also workarounds the issue through URL rewriting.

What is changed and how it works?

What's Changed:

The problem is caused by using block_on_external_io recursively. This seems to break Tokio's executor and caused the download stream to end after a single call of poll_read (where the buffer size is 8 KB).

This PR fixes the problem by removing all nested block_on_external_io and replace them by async fn.

Related changes

  • Need to cherry-pick to the release branch
    • need to cherry-pick to release-4.0.

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • Restore through fake-gcs-server works.

Side effects

Release note

Restoring a backup archive from GCS now works.

Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor Author

sre-bot commented May 29, 2020

/run-all-tests

@sre-bot sre-bot added component/backup-restore Component: backup, import, external_storage type/bugfix This PR fixes a bug. type/cherry-pick Type: PR - Cherry pick labels May 29, 2020
@sre-bot sre-bot added this to the v3.1.2 milestone May 29, 2020
Signed-off-by: kennytm <kennytm@gmail.com>
@kennytm
Copy link
Contributor

kennytm commented May 29, 2020

/run-integration-ddl-test

/run-integration-compatibility-test

@5kbpers
Copy link
Member

5kbpers commented May 29, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 29, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented May 29, 2020

Your auto merge job has been accepted, waiting for:

  • 7905

@sre-bot
Copy link
Contributor Author

sre-bot commented May 29, 2020

/run-all-tests

@5kbpers
Copy link
Member

5kbpers commented May 29, 2020

/merge

2 similar comments
@5kbpers
Copy link
Member

5kbpers commented May 29, 2020

/merge

@5kbpers
Copy link
Member

5kbpers commented May 29, 2020

/merge

@sre-bot
Copy link
Contributor Author

sre-bot commented May 29, 2020

/run-all-tests

@sre-bot
Copy link
Contributor Author

sre-bot commented May 29, 2020

@sre-bot merge failed.

@kennytm
Copy link
Contributor

kennytm commented May 29, 2020

Because of the flipped merge order, the essence of this cherry-pick has been contained in #7965 already. So this PR is much less urgent now 🤷

@5kbpers
Copy link
Member

5kbpers commented May 29, 2020

/merge

@sre-bot
Copy link
Contributor Author

sre-bot commented May 29, 2020

/run-all-tests

@sre-bot
Copy link
Contributor Author

sre-bot commented May 29, 2020

@sre-bot merge failed.

@5kbpers
Copy link
Member

5kbpers commented May 29, 2020

/merge

1 similar comment
@5kbpers
Copy link
Member

5kbpers commented May 30, 2020

/merge

@5kbpers 5kbpers modified the milestones: v3.1.2, v3.1.3 Jun 3, 2020
@CLAassistant
Copy link

CLAassistant commented Dec 16, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 3 committers have signed the CLA.

❌ kennytm
❌ 5kbpers
❌ sre-bot
You have signed the CLA already but the status is still pending? Let us recheck it.

@kennytm
Copy link
Contributor

kennytm commented Dec 16, 2020

release-3.1 is no longer maintained, closing.

@kennytm kennytm closed this Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/backup-restore Component: backup, import, external_storage status/can-merge Indicates a PR has been approved by a committer. type/bugfix This PR fixes a bug. type/cherry-pick Type: PR - Cherry pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants