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

Un-fork Azure SDK #10404

Open
arpad-m opened this issue Jan 15, 2025 · 2 comments
Open

Un-fork Azure SDK #10404

arpad-m opened this issue Jan 15, 2025 · 2 comments
Labels

Comments

@arpad-m
Copy link
Member

arpad-m commented Jan 15, 2025

Starting with #10036, we are using a forked version of the Azure SDK so that we only use one async runtime instead of two.

Blockers for un-forking:

@heaths
Copy link

heaths commented Jan 15, 2025

Note: whether using our legacy branch of what's in main now, you're not tied to a single async runtime or HTTP stack. We default to reqwest and, therefore, tokio, but you can provide your own HttpClient implementation. We've also been careful to limit production code to using abstractions from std and futures - only tests and examples should use tokio - but if you find any problems, please do file an issue.

@arpad-m
Copy link
Member Author

arpad-m commented Jan 16, 2025

The azure SDK's usage of tokio isn't the issue. We've already been heavy tokio users before, and are still such users now.

The problem is that azure_identity spawns processes using a non-tokio runtime (async-io). I have filed an issue in the SDK repo and we've already talked there, it's the first entry in the blockers list above: Azure/azure-sdk-for-rust#1652 . The process spawning should happen very rarely (in terms of CPU time eaten by the other runtime) and probably only during initialization, but we still would like to avoid the dependency on a non-tokio runtime so that we can cargo-deny it.

So in other words, the fork was made in order to include Azure/azure-sdk-for-rust#1654.


A bit unrelated, but as you mention HttpClient, this might interest you: even though we only use tokio, it's really great that we can provide a custom HttpClient implementation, that way we can configure reqwest to our liking.

We used this ability in #10169 to enable reqwest's http connection pooling (option enabled in #10324). So far the reqwest issue that motivated turning off http connection pooling in the SDK didn't manifest for us, but pooling solved an important issue for us.

Namely, before we were running into tcp port exhaustion issues on our servers because we were uploading a lot of files to Azure Blob Storage very quickly, and each upload would mean one new http connection on top of one new tcp stream, both of which would then promptly be closed. The kernel keeps the port of tcp streams allocated for a while after the stream has been closed (TIME_WAIT), to not misattribute any stray incoming packets to the new connection.

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

No branches or pull requests

2 participants