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

Object store builder blocks on IO when building #6469

Closed
chebbyChefNEQ opened this issue Sep 27, 2024 · 7 comments
Closed

Object store builder blocks on IO when building #6469

chebbyChefNEQ opened this issue Sep 27, 2024 · 7 comments
Labels
question Further information is requested

Comments

@chebbyChefNEQ
Copy link

Describe the bug
Cloud based (tested for GCS) store builder blocks on file IO synchronously in .build. This happens by:
object_store -> reqwest::ClientBuild::build -> rustls::load_native_cert

To Reproduce

use object_store::gcp::GoogleCloudStorageBuilder;

fn main() {
    for _ in 0..100 {
        let start = std::time::Instant::now();
        let _gcs = GoogleCloudStorageBuilder::from_env().with_bucket_name("some-bucket").build().unwrap();
        println!("built in: {:?}", start.elapsed());
    }
}

Expected behavior
calling build should return a future and not block

Additional context
I'll create a ticket in reqwest as well, but maybe we could capture the client build in tokio::spawn_blocking for the time being?

@chebbyChefNEQ
Copy link
Author

linux
See flame graph showing std::fs in the stack

@chebbyChefNEQ
Copy link
Author

upon further testing this is cause by the feature rustls-tls-native-roots. default-tls does not have this behavior

@chebbyChefNEQ
Copy link
Author

issue created in reqwest seanmonstar/reqwest#2437

@tustvold
Copy link
Contributor

tustvold commented Sep 27, 2024

FWIW the various object store implementations are designed to be created once and then reused, otherwise things like connection pooling, etc... won't work correctly and you will pay for non-trivial setup costs such as loading certificates.

Perhaps you could expand on the issue you are running into, it would be a very disruptive change to make the builders async, and I struggle to see what major issues performing some blocking IO on startup would cause

@chebbyChefNEQ
Copy link
Author

Hi yeah I totally get both points: 1) store should be created once and cached, and 2) making builder async is going to be a major API change.

Unfortunately :( our code makes a bit hard to do DI style constructions and we can't easily cache the store handle always. Also reqwest seems not have blocking IO behavior in its builder with other tls features. rustls-tls-native-roots seems like the only with with this issue. I def lack the context on what we need native roots maybe that's something we could change?

I guess another thing is having blocking IO here blocks our tokio worker and that causes some issues, albeit minor most of the time.

@tustvold
Copy link
Contributor

tustvold commented Sep 27, 2024

I def lack the context on what we need native roots maybe that's something we could change

There is some further context on this specific issue - #4870

TLDR you can opt-in to using webpki roots, or native-tls depending on your preferences.

our code makes a bit hard to do DI style constructions

I'd strongly encourage you to try, polars started reporting very peculiar issues a while back (pola-rs/polars#14384) that ended up being related to it spinning up hundreds of separate clients. In the end they used a global cache for this and the issues went away - pola-rs/polars#14598

@tustvold
Copy link
Contributor

tustvold commented Oct 7, 2024

I'm going to close this, as reading the linked upstream ticket I am not sure there is anything planned in this repository

@tustvold tustvold closed this as not planned Won't fix, can't repro, duplicate, stale Oct 7, 2024
@tustvold tustvold added question Further information is requested and removed bug labels Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants