feat(object store): introduce new s3 object store via OpenDAL #14409

wcy-fdu · 2024-01-08T06:28:06Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

detail in #14321

Checklist

~~- [ ] I have written necessary rustdoc comments~~
~~- [ ] I have added necessary unit tests and integration tests~~
~~- [ ] I have added test labels as necessary. See details.~~
~~- [ ] I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features #7934).~~
~~- [ ] My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).~~

All checks passed in ./risedev check (or alias, ./risedev c)
~~- [ ] My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)~~

~~- [ ] My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)~~

Documentation

~~- [ ] My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)~~

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

github-actions

license-eye has totally checked 4697 files.

Valid	Invalid	Ignored	Fixed
2049	1	2647	0

Click to see the invalid file list

src/object_store/src/object/opendal_engine/opendal_s3.rs

src/object_store/src/object/opendal_engine/opendal_s3.rs

wcy-fdu · 2024-01-19T05:58:14Z

I have verified the correctness of OpenDAL s3.

before(via aws sdk): we need to set credential in environment by aws configure or just set access_key in pod env, then sdk will call from_env() to get the credential.
For new OpenDAL S3: we need to set AWS_REGION, AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY, and for those S3 compatible storage, we need to set one more RW_S3_ENDPOINT. This is the same as current docker compose env.

Next, we need to do perf test and stability test for the new OpenDAL S3, but before that we need to apply this change in RisingWave Operator and Kube Bench. I have created an issue for this, and will start testing after this is done. So let's merge this PR first to allow other components to be easily tested. @hzxa21

The next PR will switch current minio to use OpenDAL S3.

wcy-fdu · 2024-01-30T09:30:21Z

Longevity test
https://buildkite.com/risingwave-test/longevity-test/builds/1017#018d59ab-b4c3-44b3-b075-258184cb8ae0

Nexmark perf test is running

…nto wcy/opendal_s3

wcy-fdu · 2024-02-01T09:18:44Z

Add some enhancement for opendal s3:

add concurrent write(v0.44.2)
Set the retry parameters the same as the current s3
add trust-dns feature(by bump version to 0.44.2)

…nto wcy/opendal_s3

gitguardian · 2024-02-01T11:17:07Z

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
9425213	Triggered	Generic Password	`50bc801`	ci/scripts/regress-test.sh	View secret

🛠 Guidelines to remediate hardcoded secrets

Understand the implications of revoking this secret by investigating where it is used in your code.
Replace and store your secret safely. Learn here the best practices.
Revoke and rotate this secret.
If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

following these best practices for managing and storing secrets including API keys and other credentials
install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!}

#14844)

hzxa21

It seems that the commit is messed up with a merged PR. Let's resolve it first.

Also, I don't see OpendalObjectStore::new_s3_engine being used. I thought we are going to use some flag to control the switch.

src/object_store/src/object/opendal_engine/opendal_object_store.rs

wcy-fdu · 2024-02-01T15:46:19Z

It seems that the commit is messed up with a merged PR. Let's resolve it first.

Umm... never mind, I am build RisingWave image right now, which need to be consistent with nightly0201, so I cherry pick #14844.
Will rebase main after https://buildkite.com/risingwavelabs/docker/builds/15123 is ready(blocked now)

…nto wcy/opendal_s3

src/object_store/src/object/mod.rs

src/object_store/src/object/opendal_engine/opendal_s3.rs

…nto wcy/opendal_s3 consistent with nightly-0207 for perf test.

wcy-fdu · 2024-02-08T02:14:10Z

Metabase link: http://metabase.risingwave-cloud.xyz/dashboard/241-nexmark-blackhole-1cn-anti-affinity-rw-avg-source-throughput?start_date=2024-02-07&namespace=
left:opendal-s3 right: nightly-0207

wcy-fdu · 2024-02-08T02:28:34Z

Take eyes on q104:

grafana url

opendal-s3:https://grafana.test.risingwave-cloud.xyz/d/EpkBw5W4k/risingwave-dev-dashboard?orgId=1&var-da[…]ar-namespace=nexmark-kafka-benchmark-20240207-124259
aws sdk https://grafana.test.risingwave-cloud.xyz/d/EpkBw5W4k/risingwave-dev-dashboard?orgId=1&var-da[…]74000&var-namespace=nexmark-bs-15-105-daily-20240207

metrics

streaming upload start avg
- opendal-s3
- nightly-0207

Before introducing concurrent uploading for opendal-s3, this metrics opendal side will be 100 times slower, but now both sides are basically the same.

streaming upload finish avg
- opendal-s3
- nightly-0207

Also streaming upload finish avg are basically the same now.

read p99
- opendal-s3
- nightly-0207

Previously, for this metrics(read p99/pmax), there are a lot of burrs in opendal side, we find that it's because the retry strategy is different. After turning retry config to be consistent with s3-sdk, this metrics are alse basically the same now.

Above are three abnormal metrics found in last time benchmark, and now it seems that all have been fixed or optimized. @Xuanwo

Abnormal metrics found this time

upload avg meta
- opendal-s3
- nightly-0207

Xuanwo · 2024-02-08T02:56:32Z

What does upload avg meta measure?

wcy-fdu · 2024-02-08T02:57:58Z

What does upload avg meta measure?

checkpointing

Xuanwo · 2024-02-18T05:31:39Z

Hi, @wcy-fdu, do we have a final report about this? Is there anything that opendal could improve?

wcy-fdu · 2024-02-18T05:35:05Z

Hi, @wcy-fdu, do we have a final report about this? Is there anything that opendal could improve?

We will go ahead do some perf test on main branch, comparing with everyday's nightly.
And still investigate the abnormal metricsupload avg meta, which indicate the checkpoint latency.

Xuanwo · 2024-02-18T06:13:26Z

And still investigate the abnormal metricsupload avg meta, which indicate the checkpoint latency.

Got it, let's figure out it.

Co-authored-by: William Wen <[email protected]>

#15215) Co-authored-by: congyi wang <[email protected]> Co-authored-by: William Wen <[email protected]>

wcy-fdu added 2 commits January 3, 2024 11:17

save work

a379cdb

implement new s3 via OpenDAL

4e52419

github-actions bot added the type/feature label Jan 8, 2024

github-actions bot reviewed Jan 8, 2024

View reviewed changes

src/object_store/src/object/opendal_engine/opendal_s3.rs Outdated Show resolved Hide resolved

wcy-fdu added 2 commits January 8, 2024 15:15

update Copyright

95ae846

typo

c2d8280

wcy-fdu mentioned this pull request Jan 19, 2024

Apply new object store: OpenDAL S3 risingwavelabs/risingwave-operator#579

Closed

add region

c898678

wcy-fdu requested review from hzxa21 and zwang28 January 19, 2024 05:58

kwannoel self-requested a review January 23, 2024 08:38

use env to decide whether using opendal s3

b990bad

hzxa21 requested a review from MrCroxx January 23, 2024 11:30

wcy-fdu added 2 commits January 31, 2024 11:27

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

fb42759

…nto wcy/opendal_s3

enhance opendal s3

1295931

wcy-fdu requested a review from a team as a code owner February 1, 2024 09:16

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

50bc801

…nto wcy/opendal_s3

refactor(meta): handle all stream client request in stream rpc manager (

a270d7d

#14844)

hzxa21 reviewed Feb 1, 2024

View reviewed changes

src/object_store/src/object/opendal_engine/opendal_object_store.rs Outdated Show resolved Hide resolved

wcy-fdu added 2 commits February 1, 2024 23:47

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

45a2d75

…nto wcy/opendal_s3

rollback

4438b1f

wcy-fdu commented Feb 1, 2024

View reviewed changes

src/object_store/src/object/mod.rs Outdated Show resolved Hide resolved

hzxa21 reviewed Feb 1, 2024

View reviewed changes

wcy-fdu added 2 commits February 2, 2024 00:40

minor

f1e88fd

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

881a6b6

…nto wcy/opendal_s3 consistent with nightly-0207 for perf test.

wcy-fdu requested a review from Li0k February 8, 2024 08:51

wenym1 approved these changes Feb 18, 2024

View reviewed changes

wcy-fdu enabled auto-merge February 18, 2024 05:27

MrCroxx approved these changes Feb 18, 2024

View reviewed changes

Merge branch 'main' into wcy/opendal_s3

2a524be

wcy-fdu mentioned this pull request Feb 18, 2024

chore(deps): Bump opendal from 0.44.0 to 0.45.0 #15013

Closed

TennyZhuang approved these changes Feb 20, 2024

View reviewed changes

wcy-fdu added this pull request to the merge queue Feb 20, 2024

Merged via the queue into main with commit 4197ad5 Feb 20, 2024
34 checks passed

wcy-fdu deleted the wcy/opendal_s3 branch February 20, 2024 06:44

cyliu0 mentioned this pull request Feb 21, 2024

nightly-20240220 nexmark perf degradation #15169

Closed

wcy-fdu added the need-cherry-pick-release-1.7 label Feb 23, 2024

github-actions bot pushed a commit that referenced this pull request Feb 23, 2024

feat(object store): introduce new s3 object store via OpenDAL (#14409)

12ec50b

Co-authored-by: William Wen <[email protected]>

github-actions bot mentioned this pull request Feb 23, 2024

feat(object store): introduce new s3 object store via OpenDAL (#14409) #15215

Merged

lmatz pushed a commit that referenced this pull request Feb 23, 2024

feat(object store): introduce new s3 object store via OpenDAL (#14409) (

4b1b616

#15215) Co-authored-by: congyi wang <[email protected]> Co-authored-by: William Wen <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(object store): introduce new s3 object store via OpenDAL #14409

feat(object store): introduce new s3 object store via OpenDAL #14409

wcy-fdu commented Jan 8, 2024 •

edited

Loading

github-actions bot left a comment

wcy-fdu commented Jan 19, 2024 •

edited

Loading

wcy-fdu commented Jan 30, 2024 •

edited

Loading

wcy-fdu commented Feb 1, 2024

gitguardian bot commented Feb 1, 2024 •

edited

Loading

hzxa21 left a comment

wcy-fdu commented Feb 1, 2024 •

edited

Loading

wcy-fdu commented Feb 8, 2024

wcy-fdu commented Feb 8, 2024 •

edited

Loading

Xuanwo commented Feb 8, 2024

wcy-fdu commented Feb 8, 2024 •

edited

Loading

Xuanwo commented Feb 18, 2024

wcy-fdu commented Feb 18, 2024

Xuanwo commented Feb 18, 2024

feat(object store): introduce new s3 object store via OpenDAL #14409

feat(object store): introduce new s3 object store via OpenDAL #14409

Conversation

wcy-fdu commented Jan 8, 2024 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Release note

github-actions bot left a comment

Choose a reason for hiding this comment

wcy-fdu commented Jan 19, 2024 • edited Loading

wcy-fdu commented Jan 30, 2024 • edited Loading

wcy-fdu commented Feb 1, 2024

gitguardian bot commented Feb 1, 2024 • edited Loading

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

hzxa21 left a comment

Choose a reason for hiding this comment

wcy-fdu commented Feb 1, 2024 • edited Loading

wcy-fdu commented Feb 8, 2024

wcy-fdu commented Feb 8, 2024 • edited Loading

Take eyes on q104:

grafana url

metrics

Xuanwo commented Feb 8, 2024

wcy-fdu commented Feb 8, 2024 • edited Loading

Xuanwo commented Feb 18, 2024

wcy-fdu commented Feb 18, 2024

Xuanwo commented Feb 18, 2024

wcy-fdu commented Jan 8, 2024 •

edited

Loading

wcy-fdu commented Jan 19, 2024 •

edited

Loading

wcy-fdu commented Jan 30, 2024 •

edited

Loading

gitguardian bot commented Feb 1, 2024 •

edited

Loading

wcy-fdu commented Feb 1, 2024 •

edited

Loading

wcy-fdu commented Feb 8, 2024 •

edited

Loading

wcy-fdu commented Feb 8, 2024 •

edited

Loading