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

Merged
merged 6 commits into from May 6, 2020

Conversation

kennytm
Copy link
Contributor

@kennytm kennytm commented May 3, 2020

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: kennytm <kennytm@gmail.com>
Signed-off-by: kennytm <kennytm@gmail.com>
Signed-off-by: kennytm <kennytm@gmail.com>
@kennytm kennytm added type/bugfix Type: PR - Fix a bug component/backup-restore Component: backup, import, external_storage status/PTAL Status: Waiting for reviewing needs-cherry-pick-release-4.0 Type: Need cherry pick to release 4.0 labels May 3, 2020
@kennytm
Copy link
Contributor Author

kennytm commented May 3, 2020

/release

@kennytm
Copy link
Contributor Author

kennytm commented May 3, 2020

cc @shuijing198799

Copy link
Contributor

@yiwu-arbug yiwu-arbug left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix.

Copy link
Member

@overvenus overvenus 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 added status/LGT2 Status: PR - There are already 2 approvals and removed status/PTAL Status: Waiting for reviewing labels May 6, 2020
@kennytm
Copy link
Contributor Author

kennytm commented May 6, 2020

/merge

@sre-bot sre-bot added the status/can-merge Status: Can merge to base branch label May 6, 2020
@sre-bot
Copy link
Contributor

sre-bot commented May 6, 2020

Your auto merge job has been accepted, waiting for:

  • 7729

@sre-bot
Copy link
Contributor

sre-bot commented May 6, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented May 6, 2020

@kennytm merge failed.

@kennytm
Copy link
Contributor Author

kennytm commented May 6, 2020

https://internal.pingcap.net/idc-jenkins/blue/organizations/jenkins/tikv_ghpr_integration_common_test/detail/tikv_ghpr_integration_common_test/13583/pipeline

udf_logic_xor failed
[2020-05-06T07:04:10.197Z] 2020/05/06 15:04:10.111 main.go:713: [fatal] run test [udf_logic_xor] err: sql:SELECT a, b, a xor b FROM (SELECT `pk` AS a, `col_decimal_6_3_signed` AS b FROM `table50_int_autoinc`) tmp WHERE (a xor b) = 1 ORDER BY LOWER(a), LOWER(b);: failed to run query 
[2020-05-06T07:04:10.197Z] "SELECT a, b, a xor b FROM (SELECT `pk` AS a, `col_decimal_6_3_signed` AS b FROM `table50_int_autoinc`) tmp WHERE (a xor b) = 1 ORDER BY LOWER(a), LOWER(b);" 
[2020-05-06T07:04:10.197Z]  around line 551, 
[2020-05-06T07:04:10.197Z] we need(297):
[2020-05-06T07:04:10.197Z] SELECT a, b, a xor b FROM (SELECT `pk` AS a, `col_decimal_6_3_signed` AS b FROM `table50_int_autoinc`) tmp WHERE (a xor b) = 1 ORDER BY LOWER(a), LOWER(b);
[2020-05-06T07:04:10.197Z] a	b	a xor b
[2020-05-06T07:04:10.197Z] 1	0.000	1
[2020-05-06T07:04:10.197Z] 10	0.000	1
[2020-05-06T07:04:10.198Z] 14	0.112	1
[2020-05-06T07:04:10.198Z] 16	0.000	1
[2020-05-06T07:04:10.198Z] 17	0.000	1
[2020-05-06T07:04:10.198Z] 24	0.000	1
[2020-05-06T07:04:10.198Z] 26	0.000	1
[2020-05-06T07:04:10.198Z] 29	0.000	1
[2020-05-06T07:04:10.198Z] 35	0.000	1
[2020-05-06T07:04:10.198Z] 38	0.000	1
[2020-05-06T07:04:10.198Z] 4	0.000	1
[2020-05-06T07:04:10.198Z] 41	0.112	1
[2020-05-06T07:04:10.198Z] but got(297):
[2020-05-06T07:04:10.198Z] SELECT a, b, a xor b FROM (SELECT `pk` AS a, `col_decimal_6_3_signed` AS b FROM `table50_int_autoinc`) tmp WHERE (a xor b) = 1 ORDER BY LOWER(a), LOWER(b);
[2020-05-06T07:04:10.198Z] a	b	a xor b
[2020-05-06T07:04:10.198Z] 1	0.000	1
[2020-05-06T07:04:10.198Z] 10	0.000	1
[2020-05-06T07:04:10.198Z] 16	0.000	1
[2020-05-06T07:04:10.198Z] 17	0.000	1
[2020-05-06T07:04:10.198Z] 24	0.000	1
[2020-05-06T07:04:10.198Z] 26	0.000	1
[2020-05-06T07:04:10.198Z] 29	0.000	1
[2020-05-06T07:04:10.198Z] 35	0.000	1
[2020-05-06T07:04:10.198Z] 38	0.000	1
[2020-05-06T07:04:10.198Z] 4	0.000	1
[2020-05-06T07:04:10.198Z] 45	0.000	1
[2020-05-06T07:04:10.198Z] 8	0.000	1

@kennytm
Copy link
Contributor Author

kennytm commented May 6, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented May 6, 2020

Your auto merge job has been accepted, waiting for:

  • 7729

@sre-bot
Copy link
Contributor

sre-bot commented May 6, 2020

/run-all-tests

@sre-bot sre-bot merged commit 1bdab7b into tikv:master May 6, 2020
sre-bot pushed a commit to sre-bot/tikv that referenced this pull request May 6, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented May 6, 2020

cherry pick to release-4.0 in PR #7739

@kennytm kennytm deleted the fix-gcs branch May 6, 2020 07:21
c1ay pushed a commit to c1ay/tikv that referenced this pull request May 9, 2020
…refactoring (tikv#7734)

Signed-off-by: kennytm <kennytm@gmail.com>
sre-bot added a commit that referenced this pull request May 11, 2020
…refactoring (#7734) (#7739)

Signed-off-by: sre-bot <sre-bot@pingcap.com>
@overvenus overvenus added needs-cherry-pick-release-3.1 Type: Need cherry pick to release 3.1 and removed needs-cherry-pick-release-4.0 Type: Need cherry pick to release 4.0 labels May 29, 2020
@overvenus
Copy link
Member

/run-cherry-picker

sre-bot pushed a commit to sre-bot/tikv that referenced this pull request May 29, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented May 29, 2020

cherry pick to release-3.1 in PR #7962

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 needs-cherry-pick-release-3.1 Type: Need cherry pick to release 3.1 status/can-merge Status: Can merge to base branch status/LGT2 Status: PR - There are already 2 approvals type/bugfix Type: PR - Fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BR -> GCS report download error
4 participants