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

mixed sync & async IO usage in GCS upload #415

Open
danburkert opened this issue Mar 8, 2023 · 2 comments
Open

mixed sync & async IO usage in GCS upload #415

danburkert opened this issue Mar 8, 2023 · 2 comments

Comments

@danburkert
Copy link

The google-storage1 APIs use a mix of sync (std::io) and async APIs. For instance, the upload method takes an argument of ReadSeek which uses blocking IO, but the method itself is async. Looking through the implementation, it appears that ultimately the work is done in doit, which uses the synchronous ReadSeek instance without using something like tokio::task::block_in_place. My understanding of tokio is that such mixed sync and async usage is highly discouraged, since it will end up blocking unrelated async tasks while doing the sync IO operations. Am I missing something, or is this a real issue?

@Byron
Copy link
Owner

Byron commented Mar 8, 2023

That's a great catch! When the library was converted to async, this trait was overlooked. Fixing it could certainly be done by switching it out with its async counterpart.

A PR with a fix would be greatly appreciated.

@MOZGIII
Copy link

MOZGIII commented Jun 15, 2023

Note the same issue is present at google drive API - upload there uses sync IO for reading the files.

The simplest way to refactor this seems to kill the ReadSeek trait (which exposes the sync std::io::Read + std::io::Seek) and see what blows up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants