storage: support kingsoft s3 (ks3) #47465

lance6716 · 2023-10-08T12:06:58Z

What problem does this PR solve?

Issue Number: close #47230

Problem Summary:

What is changed and how it works?

when URL scheme is ks3, use ks3 SDK rather than AWS SDK. Note that a main difference is ks3 SDK does not expose context on API ks3sdklib/aws-sdk-go#29 , so I manually set a 5 minutes timeout on HTTP client.

Check List

Tests

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

|  30001 | ks3://data0/test.item_core.10*.csv?access-key=xxxxxx&secret-access-key=xxxxxx&endpoint=http%3a%2f%2fks3-cn-beijing-internal.ksyuncs.com&force-path-style=false&region=Beijing&provider=ks
| `test`.`item_core` |      112 |            | finished | 158.8GB          |      11100000 |                | 2023-10-09 05:27:28.744758 | 2023-10-09 05:27:31.897201 | 2023-10-09 05:50:52.899005 | root@%     |

No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Signed-off-by: lance6716 <[email protected]>

tiprow · 2023-10-08T12:07:22Z

Hi @lance6716. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: lance6716 <[email protected]>

hawkingrei · 2023-10-08T15:00:26Z

/retest

codecov · 2023-10-08T15:17:38Z

Codecov Report

Merging #47465 (7525f76) into master (1b4383a) will increase coverage by 0.8238%.
The diff coverage is 12.4260%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #47465        +/-   ##
================================================
+ Coverage   72.0288%   72.8527%   +0.8238%     
================================================
  Files          1396       1420        +24     
  Lines        404192     411065      +6873     
================================================
+ Hits         291135     299472      +8337     
+ Misses        93465      92763       -702     
+ Partials      19592      18830       -762

Flag	Coverage Δ
integration	`41.1898% <0.0000%> (?)`
unit	`71.9566% <12.4260%> (-0.0723%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`53.9913% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`48.7156% <12.0792%> (-4.6474%)`	⬇️

Signed-off-by: lance6716 <[email protected]>

okJiang

This pr need a release note?

br/pkg/storage/ks3.go

Signed-off-by: lance6716 <[email protected]>

lance6716 · 2023-10-09T07:33:41Z

This pr need a release note?

Currently this is only used for testing, ks3 SDK is lacking necessary features like golang context

br/pkg/storage/ks3.go

Signed-off-by: lance6716 <[email protected]>

D3Hunter · 2023-10-09T11:02:39Z

ks3 url require redaction of AK/SK

Signed-off-by: lance6716 <[email protected]>

D3Hunter · 2023-10-09T16:13:54Z

br/pkg/storage/ks3.go

+
+const (
+	// ks3 sdk does not expose context, we use hardcoded timeout for network request
+	ks3HTTPDeadline = 5 * time.Minute


if global sort upload a 10g+ file, if failed, is retry time included?

ti-chi-bot · 2023-10-10T07:12:28Z

[LGTM Timeline notifier]

Timeline:

2023-10-09 16:17:20.964681885 +0000 UTC m=+1069038.551792031: ☑️ agreed by D3Hunter.
2023-10-10 07:12:27.246125938 +0000 UTC m=+1122744.833236105: ☑️ agreed by okJiang.

hawkingrei · 2023-10-12T17:09:36Z

/retest

ti-chi-bot · 2023-10-13T07:58:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, Leavrth, okJiang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [D3Hunter,Leavrth,okJiang]
~~br/OWNERS~~ [Leavrth]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lance6716 · 2023-10-13T09:09:18Z

/retest

tiprow · 2023-10-13T09:09:50Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lance6716 · 2023-10-13T09:40:48Z

/retest

tiprow · 2023-10-13T09:41:22Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lance6716 · 2023-10-13T12:33:34Z

/retest

tiprow · 2023-10-13T12:34:16Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

hawkingrei · 2023-10-13T13:34:25Z

/hold

lance6716 · 2023-10-14T09:33:26Z

/unhold

hawkingrei · 2023-10-15T04:50:49Z

/retest

close pingcap#47230

lance6716 added 2 commits October 8, 2023 17:36

--wip-- [skip ci]

4604d00

storage: support kingsoft s3 (ks3)

823353b

Signed-off-by: lance6716 <[email protected]>

fix bazel

e950fc9

Signed-off-by: lance6716 <[email protected]>

lance6716 added 3 commits October 9, 2023 10:18

fix bazel

34dbd2d

Signed-off-by: lance6716 <[email protected]>

Merge branch 'master' of github.com:pingcap/tidb into ks3

27e0030

add UT

a34e900

Signed-off-by: lance6716 <[email protected]>

lance6716 changed the title ~~[WIP] storage: support kingsoft s3 (ks3)~~ storage: support kingsoft s3 (ks3) Oct 9, 2023

ti-chi-bot bot removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/needs-tests-checked labels Oct 9, 2023

revert a test change

02c27dd

Signed-off-by: lance6716 <[email protected]>

okJiang reviewed Oct 9, 2023

View reviewed changes

address comment

74372c1

Signed-off-by: lance6716 <[email protected]>

okJiang reviewed Oct 9, 2023

View reviewed changes

lance6716 added 2 commits October 9, 2023 17:03

address comment

0a0980b

Signed-off-by: lance6716 <[email protected]>

ununsed ctx

2022351

Signed-off-by: lance6716 <[email protected]>

address comment

c388d82

Signed-off-by: lance6716 <[email protected]>

lance6716 requested a review from a team as a code owner October 9, 2023 13:41

D3Hunter approved these changes Oct 9, 2023

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Oct 9, 2023

okJiang approved these changes Oct 10, 2023

View reviewed changes

ti-chi-bot bot added the lgtm label Oct 10, 2023

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Oct 10, 2023

Leavrth approved these changes Oct 13, 2023

View reviewed changes

ti-chi-bot bot added the approved label Oct 13, 2023

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 13, 2023

Merge branch 'master' of github.com:pingcap/tidb into ks3

7525f76

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2023

ti-chi-bot bot merged commit 16f3e79 into pingcap:master Oct 15, 2023

budney pushed a commit to budney/tidb that referenced this pull request May 18, 2025

storage: support kingsoft s3 (ks3) (pingcap#47465)

7ad1208

close pingcap#47230

storage: support kingsoft s3 (ks3) #47465

storage: support kingsoft s3 (ks3) #47465

Uh oh!

Conversation

lance6716 commented Oct 8, 2023 • edited by D3Hunter Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

tiprow bot commented Oct 8, 2023

Uh oh!

hawkingrei commented Oct 8, 2023

Uh oh!

codecov bot commented Oct 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

okJiang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lance6716 commented Oct 9, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

D3Hunter commented Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

D3Hunter Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Oct 10, 2023

[LGTM Timeline notifier]

Uh oh!

hawkingrei commented Oct 12, 2023

Uh oh!

ti-chi-bot bot commented Oct 13, 2023

Uh oh!

lance6716 commented Oct 13, 2023

Uh oh!

tiprow bot commented Oct 13, 2023

Uh oh!

lance6716 commented Oct 13, 2023

Uh oh!

tiprow bot commented Oct 13, 2023

Uh oh!

lance6716 commented Oct 13, 2023

Uh oh!

tiprow bot commented Oct 13, 2023

Uh oh!

hawkingrei commented Oct 13, 2023

Uh oh!

lance6716 commented Oct 14, 2023

Uh oh!

hawkingrei commented Oct 15, 2023

Uh oh!

Uh oh!

lance6716 commented Oct 8, 2023 •

edited by D3Hunter

Loading

codecov bot commented Oct 8, 2023 •

edited

Loading

D3Hunter commented Oct 9, 2023 •

edited

Loading