feat: Introduce Google Cloud Storage Implementation #4344

Xuanwo · 2024-01-03T08:32:19Z

Description

Part of #4236

This PR implements:

a basic opendal storage implementation (not exposed)
Google Cloud Storage Implementation (exposed via config)

There are many TODOs to finish, but I feel like it's better to make it working first.

How was this PR tested?

Added google_cloud_storage_test_suite

Signed-off-by: Xuanwo <github@xuanwo.io>

CLAassistant · 2024-01-03T08:32:26Z

All committers have signed the CLA.

quickwit/quickwit-storage/src/opendal_storage.rs

quickwit/Cargo.toml

Signed-off-by: Xuanwo <github@xuanwo.io>

Xuanwo · 2024-01-04T08:10:09Z

Hi @etolbakov, what are your thoughts on the current plan? Should we first merge the OpenDAL storage and then add GCS storage? For GCS, integration with configurations and settings is necessary. Once GCS is operational, we can incorporate metrics and other utilities similar to those provided by S3 or FS.

etolbakov · 2024-01-04T12:02:21Z

Hey @Xuanwo
Actually I was doing some work on this ticket in the background, to my shame I didn't assign/mention that anyhow.
My implementation strategy was the opposite: using opendal exactly for gcs integration, make sure it fits nicely in the project, then make it generic and potentially migrate other storage layers.
But I totally see where you are coming from. So we may join our efforts.

but I feel like it's better to make it working first.

Could you please elaborate a bit about what do you mean by "working first". Is it a simple integration test that we can draft?

Feel free to reach out in the discord if that's fine with you

Xuanwo · 2024-01-05T02:22:18Z

Feel free to reach out in the discord if that's fine with you

Great, moved to https://discord.com/channels/908281611840282624/908281611840282627/1192630218482012180

Signed-off-by: Xuanwo <github@xuanwo.io>

…to add-gcs-support

Signed-off-by: Xuanwo <github@xuanwo.io>

Xuanwo · 2024-01-08T08:07:57Z

Hi @etolbakov, could you share your Discord ID? I'd like to invite you to our discussion thread: https://discord.com/channels/908281611840282624/1192630218482012180

fulmicoton · 2024-01-08T09:57:49Z

@Xuanwo I like the approach: using a generic opendal storage, and adding a factory for googlefs.
I should have time tomorrow to properly review.

quickwit/quickwit-storage/src/opendal_storage/base.rs

quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs

Signed-off-by: Xuanwo <github@xuanwo.io>

fulmicoton · 2024-01-09T02:02:06Z

quickwit/Cargo.toml

@@ -118,6 +118,7 @@ num_cpus = "1"
 numfmt = "1.1.1"
 once_cell = "1"
 oneshot = "0.1.5"
+opendal = { version ="0.44", default-features = false }


can we feature gate this to make the development a tiny bit lighter?
(You can mimick what is done for azure.)

I've added a new feature called google. Is that what you were looking for?

quickwit/quickwit-config/src/storage_config.rs

quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs

quickwit/quickwit-storage/src/opendal_storage/base.rs

Signed-off-by: Xuanwo <github@xuanwo.io>

quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs

quickwit/quickwit-storage/src/opendal_storage/base.rs

fulmicoton

Do you know a way to mock gfs for our CI? Also have you seen the storage testsuite?

Signed-off-by: Xuanwo <github@xuanwo.io>

Xuanwo · 2024-01-09T02:51:51Z

Do you know a way to mock gfs for our CI? Also have you seen the storage testsuite?

I have added fake-gcs-server in CI and docker compose, but I don't know how get it started.

Xuanwo · 2024-01-09T02:58:56Z

I have added fake-gcs-server in CI and docker compose, but I don't know how get it started.

Ok, I got the right place. Let me add them.

.github/workflows/coverage.yml

Signed-off-by: Xuanwo <github@xuanwo.io>

Xuanwo · 2024-01-09T05:07:32Z

I have added integration tests for gcs:

test google_cloud_storage_test_suite ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.19s

The mock gcs server doesn't properly support bulk deletion, so I've disabled that test. However, I'm confident it functions correctly on the actual gcs.

quickwit/Cargo.toml

fulmicoton · 2024-01-09T06:56:05Z

quickwit/quickwit-storage/src/lib.rs

-            .context("write_and_bulk_delete")?;
+        // Fake GCS Server doesn't support bulk delete correctly.
+        // ref: <https://github.com/fsouza/fake-gcs-server/issues/1443>
+        #[cfg(not(feature = "google"))]


Let's not do the check using the compiler flag, but instead dynamically inspect the storage.
As is, your code is removing this test for azure too in our CI.

(these test are running in the confusingly named "coverage CI" that is triggered after each merge to main. That build takes a bit more time than the regular PR CI and includes azure etc.)

You can have the Storage trait implement Any.

Addresed, please review again

quickwit/quickwit-storage/src/opendal_storage/base.rs

quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs

Co-authored-by: Paul Masurel <paul@quickwit.io>

…rage.rs Co-authored-by: Paul Masurel <paul@quickwit.io>

Signed-off-by: Xuanwo <github@xuanwo.io>

quickwit/quickwit-config/src/storage_config.rs

etolbakov

@Xuanwo looks wonderful! I don't have anything to suggest!

fulmicoton · 2024-01-09T13:27:06Z

quickwit/quickwit-storage/Cargo.toml

@@ -70,11 +74,19 @@ azure = [
  "azure_storage/enable_reqwest_rustls",
  "azure_storage_blobs/enable_reqwest_rustls",
 ]
+google = [


can we call the feature gfs instead of google?

Sure, I will update this place tomorrow. 😋

i ll do it actually... i spotted a bunch of other problems.

Xuanwo · 2024-01-10T03:04:42Z

Thanks a lot!

fulmicoton · 2024-01-10T03:07:21Z

Thank you @Xuanwo that's awesome!

feat: Introduce OpenDAL Storage Implementation

6698e53

Signed-off-by: Xuanwo <github@xuanwo.io>

etolbakov reviewed Jan 3, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage.rs Outdated Show resolved Hide resolved

etolbakov reviewed Jan 3, 2024

View reviewed changes

quickwit/Cargo.toml Outdated Show resolved Hide resolved

Xuanwo added 2 commits January 4, 2024 08:20

bump copyright

d0036d7

Signed-off-by: Xuanwo <github@xuanwo.io>

Merge branch 'main' into add-gcs-support

8a40e60

feat: Add gcs support

9c5daf3

Signed-off-by: Xuanwo <github@xuanwo.io>

Xuanwo changed the title ~~feat: Introduce OpenDAL Storage Implementation~~ feat: Introduce Google Cloud Storage Implementation Jan 8, 2024

Xuanwo added 3 commits January 8, 2024 15:56

Merge remote-tracking branch 'refs/remotes/xuanwo/add-gcs-support' in…

e54a8c4

…to add-gcs-support

Merge branch 'main' into add-gcs-support

ebdb017

polish

fd2bc5b

Signed-off-by: Xuanwo <github@xuanwo.io>

fulmicoton self-requested a review January 8, 2024 09:57

etolbakov reviewed Jan 8, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/base.rs Outdated Show resolved Hide resolved

etolbakov reviewed Jan 8, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs Outdated Show resolved Hide resolved

Nit

0b59c73

Signed-off-by: Xuanwo <github@xuanwo.io>

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-config/src/storage_config.rs Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs Outdated Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/base.rs Outdated Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/base.rs Outdated Show resolved Hide resolved

Address comments

c02cffa

Signed-off-by: Xuanwo <github@xuanwo.io>

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/base.rs Outdated Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/base.rs Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/base.rs Outdated Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/base.rs Outdated Show resolved Hide resolved

fulmicoton suggested changes Jan 9, 2024

View reviewed changes

Xuanwo added 3 commits January 9, 2024 10:33

Add google support in uri and config resolver

3ed642c

Signed-off-by: Xuanwo <github@xuanwo.io>

Fix wording

2e3e501

Signed-off-by: Xuanwo <github@xuanwo.io>

Add fake gcs server support

6fdd362

Signed-off-by: Xuanwo <github@xuanwo.io>

fulmicoton reviewed Jan 9, 2024

View reviewed changes

.github/workflows/coverage.yml Outdated Show resolved Hide resolved

Setup integration tests for gcs

6721661

Signed-off-by: Xuanwo <github@xuanwo.io>

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/Cargo.toml Outdated Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/base.rs Outdated Show resolved Hide resolved

fulmicoton reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-storage/src/opendal_storage/google_cloud_storage.rs Outdated Show resolved Hide resolved

Xuanwo and others added 4 commits January 9, 2024 15:41

Update quickwit/Cargo.toml

b0185f5

Co-authored-by: Paul Masurel <paul@quickwit.io>

Update quickwit/quickwit-storage/src/opendal_storage/base.rs

d4449db

Co-authored-by: Paul Masurel <paul@quickwit.io>

Update quickwit/quickwit-storage/src/opendal_storage/google_cloud_sto…

1fd4fcc

…rage.rs Co-authored-by: Paul Masurel <paul@quickwit.io>

Polish bulk delete

be447f3

Signed-off-by: Xuanwo <github@xuanwo.io>

fulmicoton approved these changes Jan 9, 2024

View reviewed changes

Merge branch 'main' into add-gcs-support

7095944

etolbakov reviewed Jan 9, 2024

View reviewed changes

quickwit/quickwit-config/src/storage_config.rs Show resolved Hide resolved

etolbakov approved these changes Jan 9, 2024

View reviewed changes

fulmicoton reviewed Jan 9, 2024

View reviewed changes

fulmicoton mentioned this pull request Jan 10, 2024

env var override or overriden? #4355

Closed

fulmicoton merged commit d47b6bd into quickwit-oss:main Jan 10, 2024
7 checks passed

Xuanwo deleted the add-gcs-support branch January 10, 2024 03:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce Google Cloud Storage Implementation #4344

feat: Introduce Google Cloud Storage Implementation #4344

Xuanwo commented Jan 3, 2024 •

edited

Loading

CLAassistant commented Jan 3, 2024 •

edited

Loading

Xuanwo commented Jan 4, 2024

etolbakov commented Jan 4, 2024 •

edited

Loading

Xuanwo commented Jan 5, 2024

Xuanwo commented Jan 8, 2024

fulmicoton commented Jan 8, 2024

fulmicoton Jan 9, 2024

Xuanwo Jan 9, 2024

fulmicoton Jan 9, 2024

fulmicoton left a comment

Xuanwo commented Jan 9, 2024

Xuanwo commented Jan 9, 2024

Xuanwo commented Jan 9, 2024 •

edited

Loading

fulmicoton Jan 9, 2024 •

edited

Loading

Xuanwo Jan 9, 2024

etolbakov left a comment

fulmicoton Jan 9, 2024

Xuanwo Jan 9, 2024

fulmicoton Jan 10, 2024

Xuanwo commented Jan 10, 2024

fulmicoton commented Jan 10, 2024

feat: Introduce Google Cloud Storage Implementation #4344

feat: Introduce Google Cloud Storage Implementation #4344

Conversation

Xuanwo commented Jan 3, 2024 • edited Loading

Description

How was this PR tested?

CLAassistant commented Jan 3, 2024 • edited Loading

Xuanwo commented Jan 4, 2024

etolbakov commented Jan 4, 2024 • edited Loading

Xuanwo commented Jan 5, 2024

Xuanwo commented Jan 8, 2024

fulmicoton commented Jan 8, 2024

fulmicoton Jan 9, 2024

Choose a reason for hiding this comment

Xuanwo Jan 9, 2024

Choose a reason for hiding this comment

fulmicoton Jan 9, 2024

Choose a reason for hiding this comment

fulmicoton left a comment

Choose a reason for hiding this comment

Xuanwo commented Jan 9, 2024

Xuanwo commented Jan 9, 2024

Xuanwo commented Jan 9, 2024 • edited Loading

fulmicoton Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

Xuanwo Jan 9, 2024

Choose a reason for hiding this comment

etolbakov left a comment

Choose a reason for hiding this comment

fulmicoton Jan 9, 2024

Choose a reason for hiding this comment

Xuanwo Jan 9, 2024

Choose a reason for hiding this comment

fulmicoton Jan 10, 2024

Choose a reason for hiding this comment

Xuanwo commented Jan 10, 2024

fulmicoton commented Jan 10, 2024

Xuanwo commented Jan 3, 2024 •

edited

Loading

CLAassistant commented Jan 3, 2024 •

edited

Loading

etolbakov commented Jan 4, 2024 •

edited

Loading

Xuanwo commented Jan 9, 2024 •

edited

Loading

fulmicoton Jan 9, 2024 •

edited

Loading