-
Notifications
You must be signed in to change notification settings - Fork 361
Add selecting a tokio runtime flavor
section to docs, consider shifting examples to use current thread tokio runtime
#985
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
Comments
If we did want to keep the multithreaded runtime usage, we probably should explicitly spawn the service function future to a tokio task. Since, it is inefficient to run a loop delegating hot work, on the
It might even be worth documenting this best practice explicitly in crate docs. I think we probably should add a 'selecting a runtime flavor' section, which could mention this. |
selecting a tokio runtime flavor
section to docs, consider shifting examples to use current thread tokio runtime
I discussed this with some of my colleagues, and we are aligned that it's probably a bad idea to point people to current thread runtime in canonical docs or examples. While it technically might eke out some minor performance benefits for smaller lambdas running on 1vcpu, it just has so many footguns - for instance if any libraries you call use So probably implicitly guiding people towards multi-threaded is the right choice, and anyway expert users can make their own decisions. I'll still cut a PR to cut our examples over to spawning the handler future to a worker thread just since it is a general best practice. |
This issue is now closed. Comments on closed issues are hard for our team to see. |
All of our examples currently are using the multi-threaded, work stealing flavor of tokio's runtime. This probably isn't the right choice for the generic use case - after all, lambdas only process one request at a time. So the multithreaded runtime is just added overhead, hurts cold starts, etc.
Probably we should cut over to making them explicitly current threaded, and just leave a note on why they might want to cut to multi-threaded (heavy use of async concurrency, background tasks that need cpu cycles, etc).
The main downside is that the current thread runtime is very vulnerable to blocking, so it might make it easier for people to shoot themselves in the foot. We should at least leave a breadcrumb, maybe a link to Alice's blog, if we cut over?
If this isn't a change we want to make, we should probably at least explicitly depend on the multithreaded runtime feature in our examples. Today it is implicitly enabled by the dependency in
lambda_runtime
(and sometimeslambda_extension
). So the examples would break if we stripped that out per #984 . Better to be explicit in either case.The text was updated successfully, but these errors were encountered: